-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[minor] Fix JDBC doc after #2229 #2314
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
[minor] Fix JDBC doc after #2229 #2314
Conversation
|
@AhyoungRyu can you help review this PR. With that (#2229), I assumed all syntax of MD will work for documentation, but it didn't generate the table. |
docs/interpreter/jdbc.md
Outdated
| [Maven Repository : org.apache.hive:hive-jdbc](https://mvnrepository.com/artifact/org.apache.hive/hive-jdbc) | ||
|
|
||
| ##### Impersonation | ||
| When Zeppelin server is running with authentication enabled, then the interpreter can utilize Hive's user proxy feature i.e. send extra parameter for creating and running a session ("hive.server2.proxy.user=": "${loggedInUser}"). This is particularly useful when multiple users are sharing a notebooks. |
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.
I think are sharing a notebooks should be a notebook :)
docs/interpreter/jdbc.md
Outdated
| | hive.url | jdbc:hive2://hive-server-host:2181/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=hiveserver2 | | ||
| | hive.proxy.user.property | hive.proxy.user.property | | ||
| | zeppelin.jdbc.auth.type | SIMPLE | | ||
| Example configuration |
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.
Then do we need this line?
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.
Yes, I was thinking of sample/example configuration, or should I change the line below ##### Properties to ##### Sample configuration ?
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.
Yeah that would be better i think :)
docs/interpreter/jdbc.md
Outdated
| | zeppelin.jdbc.auth.type | SIMPLE | | ||
| Example configuration | ||
|
|
||
| ##### Properties |
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.
|
@AhyoungRyu Thank you for the review, have made the recommended changes. |
|
ah sorry I should have picked this up in review |
|
Thank you @AhyoungRyu, @felixcheung for the review. |
### What is this PR for? Fix JDBC doc after apache#2229. ### What type of PR is it? [Bug Fix] ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? * Is there breaking changes for older versions? * Does this needs documentation? Author: Prabhjyot Singh <[email protected]> Closes apache#2314 from prabhjyotsingh/minor/jdbc-doc-zeppelin-2367 and squashes the following commits: e54a3a2 [Prabhjyot Singh] @AhyoungRyu review comments 0f396ac [Prabhjyot Singh] fix doc for zeppelin-2367

What is this PR for?
Fix JDBC doc after #2229.
What type of PR is it?
[Bug Fix]
Screenshots (if appropriate)
Questions: