You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by GitBox <gi...@apache.org> on 2022/04/17 14:37:28 UTC

[GitHub] [zeppelin] huage1994 opened a new pull request, #4360: [ZEPPELIN-5614] A JDBC interpreter property for hive engines which don't need to set yarn application tag

huage1994 opened a new pull request, #4360:
URL: https://github.com/apache/zeppelin/pull/4360

   ### What is this PR for?
   This PR introduce a JDBC interpreter property for hive engines which don't need to set yarn application tag.
   
   Some users disabled tez.* property in hive  and hence unable to use hive in zeppelin. The reason is Zeppelin jdbc interpreter always set both tez and MR tags property when connect to hive.   This PR provide a property to disable setting tag for tez application and MR application .
   
   
   ### What type of PR is it?
   Improvement
   
   
   ### Todos
   * [ ] - Task
   
   ### What is the Jira issue?
   * Open an issue on Jira https://issues.apache.org/jira/browse/ZEPPELIN-5614
   
   ### How should this be tested?
   * CI passed
   
   ### Screenshots (if appropriate)
   
   ### Questions:
   * Does the licenses files need to update? No
   * Is there breaking changes for older versions? No
   * Does this needs documentation? No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zeppelin] huage1994 commented on a diff in pull request #4360: [ZEPPELIN-5614] A JDBC interpreter property for disable setting tag for application of hive engines

Posted by GitBox <gi...@apache.org>.
huage1994 commented on code in PR #4360:
URL: https://github.com/apache/zeppelin/pull/4360#discussion_r857061596


##########
jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java:
##########
@@ -604,6 +604,10 @@ private String appendProxyUserToURL(String url, String user, String propertyKey)
 
   // only add tags for hive jdbc
   private String appendTagsToURL(String url, InterpreterContext context) {
+    if (!Boolean.parseBoolean(getProperty("zeppelin.jdbc.hive.engines.tag.enable"))) {

Review Comment:
   > set default value as `true` in case `zeppelin.jdbc.hive.engines.tag.enable` is not set when user upgrade zeppelin from old versions
   
   Good point! 
    



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zeppelin] huage1994 commented on a diff in pull request #4360: [ZEPPELIN-5614] A JDBC interpreter property for disable setting tag for application of hive engines

Posted by GitBox <gi...@apache.org>.
huage1994 commented on code in PR #4360:
URL: https://github.com/apache/zeppelin/pull/4360#discussion_r851894977


##########
jdbc/src/main/resources/interpreter-setting.json:
##########
@@ -143,6 +143,13 @@
         "defaultValue": "1000",
         "description": "Query interval for hive statement",
         "type": "number"
+      },
+      "zeppelin.jdbc.hive.tagged.engines.exclude": {

Review Comment:
   Done. 
   By the way I fixed a problem about the type of  default value of   `"zeppelin.jdbc.auth.kerberos.proxy.enable"`.
   If it is string `"true"`  instead of  boolean `true`,  the checkbox in UI would not ✅ .



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zeppelin] zjffdu commented on a diff in pull request #4360: [ZEPPELIN-5614] A JDBC interpreter property for disable setting tag for application of hive engines

Posted by GitBox <gi...@apache.org>.
zjffdu commented on code in PR #4360:
URL: https://github.com/apache/zeppelin/pull/4360#discussion_r852185434


##########
jdbc/src/main/resources/interpreter-setting.json:
##########
@@ -143,6 +143,13 @@
         "defaultValue": "1000",
         "description": "Query interval for hive statement",
         "type": "number"
+      },
+      "zeppelin.jdbc.hive.tagged.engines.exclude": {

Review Comment:
   Also need to update `jdbc.md` for this new property. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zeppelin] zjffdu merged pull request #4360: [ZEPPELIN-5614] A JDBC interpreter property for disable setting tag for application of hive engines

Posted by GitBox <gi...@apache.org>.
zjffdu merged PR #4360:
URL: https://github.com/apache/zeppelin/pull/4360


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zeppelin] huage1994 commented on a diff in pull request #4360: [ZEPPELIN-5614] A JDBC interpreter property for disable setting tag for application of hive engines

Posted by GitBox <gi...@apache.org>.
huage1994 commented on code in PR #4360:
URL: https://github.com/apache/zeppelin/pull/4360#discussion_r856872638


##########
jdbc/src/main/resources/interpreter-setting.json:
##########
@@ -143,6 +143,13 @@
         "defaultValue": "1000",
         "description": "Query interval for hive statement",
         "type": "number"
+      },
+      "zeppelin.jdbc.hive.tagged.engines.exclude": {

Review Comment:
   Hi @zjffdu , I have put all hive-related properties into `hive.md` and add a link to `hive.md` in `jdbc.md`. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zeppelin] huage1994 commented on a diff in pull request #4360: [ZEPPELIN-5614] A JDBC interpreter property for disable setting tag for application of hive engines

Posted by GitBox <gi...@apache.org>.
huage1994 commented on code in PR #4360:
URL: https://github.com/apache/zeppelin/pull/4360#discussion_r851858531


##########
jdbc/src/main/resources/interpreter-setting.json:
##########
@@ -143,6 +143,13 @@
         "defaultValue": "1000",
         "description": "Query interval for hive statement",
         "type": "number"
+      },
+      "zeppelin.jdbc.hive.tagged.engines.exclude": {

Review Comment:
   Get it ! 😀 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zeppelin] zjffdu commented on a diff in pull request #4360: [ZEPPELIN-5614] A JDBC interpreter property for disable setting tag for application of hive engines

Posted by GitBox <gi...@apache.org>.
zjffdu commented on code in PR #4360:
URL: https://github.com/apache/zeppelin/pull/4360#discussion_r852612929


##########
jdbc/src/main/resources/interpreter-setting.json:
##########
@@ -143,6 +143,13 @@
         "defaultValue": "1000",
         "description": "Query interval for hive statement",
         "type": "number"
+      },
+      "zeppelin.jdbc.hive.tagged.engines.exclude": {

Review Comment:
   Both places should update. There's one section about hive in `jdbc.md`, https://zeppelin.apache.org/docs/0.10.1/interpreter/jdbc.html#apache-hive. BTW, it looks like to we need to converge the hive related doc in these 2 places. But it could be done in another PR. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zeppelin] zjffdu commented on pull request #4360: [ZEPPELIN-5614] A JDBC interpreter property for disable setting tag for application of hive engines

Posted by GitBox <gi...@apache.org>.
zjffdu commented on PR #4360:
URL: https://github.com/apache/zeppelin/pull/4360#issuecomment-1109737678

   Will merge if no more comment


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zeppelin] zjffdu commented on a diff in pull request #4360: [ZEPPELIN-5614] A JDBC interpreter property for disable setting tag for application of hive engines

Posted by GitBox <gi...@apache.org>.
zjffdu commented on code in PR #4360:
URL: https://github.com/apache/zeppelin/pull/4360#discussion_r851839988


##########
jdbc/src/main/resources/interpreter-setting.json:
##########
@@ -143,6 +143,13 @@
         "defaultValue": "1000",
         "description": "Query interval for hive statement",
         "type": "number"
+      },
+      "zeppelin.jdbc.hive.tagged.engines.exclude": {

Review Comment:
   I think this could be a boolean property, it is not necessary to differentiate mr or tez, because the user can switch engine easily. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zeppelin] zjffdu commented on a diff in pull request #4360: [ZEPPELIN-5614] A JDBC interpreter property for disable setting tag for application of hive engines

Posted by GitBox <gi...@apache.org>.
zjffdu commented on code in PR #4360:
URL: https://github.com/apache/zeppelin/pull/4360#discussion_r856894290


##########
jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java:
##########
@@ -604,6 +604,10 @@ private String appendProxyUserToURL(String url, String user, String propertyKey)
 
   // only add tags for hive jdbc
   private String appendTagsToURL(String url, InterpreterContext context) {
+    if (!Boolean.parseBoolean(getProperty("zeppelin.jdbc.hive.engines.tag.enable"))) {

Review Comment:
   set default value as `true` in case `zeppelin.jdbc.hive.engines.tag.enable` is not set when user upgrade zeppelin from old versions



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [zeppelin] huage1994 commented on a diff in pull request #4360: [ZEPPELIN-5614] A JDBC interpreter property for disable setting tag for application of hive engines

Posted by GitBox <gi...@apache.org>.
huage1994 commented on code in PR #4360:
URL: https://github.com/apache/zeppelin/pull/4360#discussion_r852553473


##########
jdbc/src/main/resources/interpreter-setting.json:
##########
@@ -143,6 +143,13 @@
         "defaultValue": "1000",
         "description": "Query interval for hive statement",
         "type": "number"
+      },
+      "zeppelin.jdbc.hive.tagged.engines.exclude": {

Review Comment:
   > Also need to update `jdbc.md` for this new property.
   
   Hi @zjffdu , This new property is only related to hive, should I put it into `hive.md` ?   
   I didn't find a good place for this property in `jdbc.md`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org