-
Notifications
You must be signed in to change notification settings - Fork 2.8k
ZEPPELIN-440 HiveInterpreter with multiple configuration #455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Like the idea about '('. |
|
@Leemoonsoo Thanks. If you have any new or more Zeppelinous idea for specifying properties, feel free to talk. I'll make it pass to CI and add docs for this function. |
|
It's nice. Would it be possible to make the interpreter group/name fully customizable? |
|
User can multiple settings (profiles) for an interpreter, in spark-dev settings can be made for the same %spark interpreter. Current restriction is, only one interpreter setting can be used in the single notebook, in the same time. If we use %[setting name] for selecting interpreter, Pros:
Cons:
I think, in the future, we need to invent some way to allow all 'settings' being used in the same notebook at the same time, without having Cons that i mentioned above. Until that time, I think this approach could be useful. |
|
Good point - maybe we should save settings with notebook to make it portable? |
e3a1f2a to
0fd5e7f
Compare
|
@felixcheung First time, I also try to run interpreter by %{setting name}, but ... If it's possible to make notebook portable with settings, %{setting name} would be better. |
|
trigger the CI again |
|
travis fails while downloading dependencies |
|
@Leemoonsoo Could you please check the travis log? I think this is about ZeppelinSparkClusterTest. Do you have any idea? |
dc2803a to
6659c35
Compare
|
this failure depends on #468 |
|
I revised HiveInterpreter.executeSql from PostgresSqlInterpreter.executeSql. I think that would be more general way to implement |
|
@Leemoonsoo @felixcheung Please review this PR, again. I've almost done. Additionally, @Leemoonsoo, after review, can I try to merge this PR into apache master branch by myself? If it's possible, could you please tell me a procedure for it? |
|
@jongyoul merge a PR is very simple, you just:
|
|
About merging, you can use dev/merge_zeppelin_pr.py script. I suggest clone zeppelin git repo into separate dir (separate from development) and then you'll need to add remote repo 'apache' and 'apache-github' to use this script. Once it's done, just run dev/merge_zeppelin_pr.py will guide you merge the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think, does horizontal alignment makes it more readable or just makes it longer?
According to the styleguide Zeppelin project uses - it is never required and has some cons https://google.github.io/styleguide/javaguide.html#s4.6.3-horizontal-alignment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, I think it doesn't need except when that one line violates the maximum number of columns. I'll fix this, too.
|
👍 for test and docs! |
|
@bzz Could you please verify this finally? |
docs/interpreter/hive.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: Zeppelin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parameterization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also use this relative path for the link (url has changed for multiple releases)
../manual/dynamicform.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or {{BASE_PATH}}/manual/dynamicform.html would work
docs/interpreter/hive.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: wih -> with
|
You maybe also want to change a link https://github.com/apache/incubator-zeppelin/blob/master/docs/_includes/themes/zeppelin/_navigation.html#L41 |
- Enable multiple connection properties
- Revert log4j.properties
- Remove commented codes - Fixed license part
- Fixed linefeed to variable
- Fixed some tests
- Fixed getScriptBody return after trimming
- Restored some tests from the beginning
- Restored ParallelScheduler from FIFOScheduler
- Revised executeSql from Postgresql.executeSql
- Fixed test case to be more general
- Fixed test case to be more general
- Updated docs for hive
- Fixed Apaceh license header - Fixed some codestyle - Removed unused codes and classes
- Fixed typos on docs
- Rebased and fixed links of docs
6f55e3e to
af4c64b
Compare
|
@Leemoonsoo I've rebased and fixed it |
|
Merging if there is no more discussion. |

This PR enable HiveInterpreter to configure several connection information. For using specific configurations, you should use some tricks like
%hive(configuration_prefix). without it, you can use default settings starting withdefault.