Skip to content

Conversation

@zjffdu
Copy link
Contributor

@zjffdu zjffdu commented Sep 19, 2016

What is this PR for?

here're several ways to load interpreter-setting.json, but for now we will load it multiple times. It is supposed to load only once. We should load it by the following orders
* 1. Register it from path {ZEPPELIN_HOME}/interpreter/{interpreter_name}/ interpreter-setting.json
* 2. Register it from interpreter-setting.json in classpath {ZEPPELIN_HOME}/interpreter/{interpreter_name}
* 3. Register it by Interpreter.register

What type of PR is it?

[Bug Fix]

Todos

  • - Task

What is the Jira issue?

How should this be tested?

Check the log that each interpreter is registered once. And also modify file interpreter/spark/interpreter-setting.json to make pyspark as the default interpreter and it works. Before this PR, it doesn't work, because it would be override by interpreter-setting.json in interpreter/spark/zeppelin-spark_2.10-0.7.0-SNAPSHOT.jar

Screenshots (if appropriate)

image

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

logger.warn("Static initialization is deprecated for interpreter {}, You should change it " +
"to use interpreter-setting.json in your jar or " +
"interpreter/{interpreter}/interpreter-setting.json", name);
register(new RegisteredInterpreter(name, group, className, defaultInterpreter, properties));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just improve the logging.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

@zjffdu
Copy link
Contributor Author

zjffdu commented Sep 19, 2016

@jongyoul Could you help review it ? Thanks

@jongyoul
Copy link
Member

Thanks Jeff, this patch fixes my mistake correctly. LGTM. If CI passes, I'll merge it asap.

@zjffdu
Copy link
Contributor Author

zjffdu commented Sep 19, 2016

@jongyoul The test failure seems irrelevant. I create 2 tickets for these flaky test.

@asfgit asfgit closed this in 9b84218 Sep 20, 2016
pedrozatta pushed a commit to pedrozatta/zeppelin that referenced this pull request Oct 27, 2016
### What is this PR for?
here're several ways to load interpreter-setting.json, but for now we will load it multiple times. It is supposed to load only once. We should load it by the following orders
         * 1. Register it from path {ZEPPELIN_HOME}/interpreter/{interpreter_name}/ interpreter-setting.json
         * 2. Register it from interpreter-setting.json in classpath {ZEPPELIN_HOME}/interpreter/{interpreter_name}
         * 3. Register it by Interpreter.register

### What type of PR is it?
[Bug Fix]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-1368

### How should this be tested?
Check the log that each interpreter is registered once. And also modify file interpreter/spark/interpreter-setting.json to make pyspark as the default interpreter and it works. Before this PR, it doesn't work, because it would be override by interpreter-setting.json in `interpreter/spark/zeppelin-spark_2.10-0.7.0-SNAPSHOT.jar`

### Screenshots (if appropriate)
![image](https://cloud.githubusercontent.com/assets/164491/18621557/a4510966-7e57-11e6-8c9a-80697ebf2600.png)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Jeff Zhang <[email protected]>

Closes apache#1435 from zjffdu/ZEPPELIN-1368 and squashes the following commits:

8266d12 [Jeff Zhang] ZEPPELIN-1368. interpreter-setting.json may be loaded mutliple times
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants