You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/08/26 12:53:38 UTC
[GitHub] [iceberg] hehuiyuan opened a new pull request #3034: [ICEBERG-FLINK]support read hive configuration from HIVE_HOME&HIVE_CONF_DIR env
hehuiyuan opened a new pull request #3034:
URL: https://github.com/apache/iceberg/pull/3034
Add load hive configuration from HIVE_CONF_DIR or HIVE_HOME
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] hehuiyuan edited a comment on pull request #3034: [ICEBERG-FLINK]support read hive configuration from HIVE_HOME&HIVE_CONF_DIR env
Posted by GitBox <gi...@apache.org>.
hehuiyuan edited a comment on pull request #3034:
URL: https://github.com/apache/iceberg/pull/3034#issuecomment-908311617
Hi @kbendick , is that what you mean?
Take a look when you have time.?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #3034: [ICEBERG-FLINK]support read hive configuration from HIVE_HOME&HIVE_CONF_DIR env
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #3034:
URL: https://github.com/apache/iceberg/pull/3034#issuecomment-907878130
> Hi @kbendick , thanks.
> Update.
Hi @hehuiyuan, thanks for clarifying the reasoning for this. I'd have one suggestion. The `mergeHiveConf` method is getting a little large. Could you possibly create a method like `hiveConfFromEnvVariables` or something and then call that in `mergeHiveConf`?
I know the current `mergeHiveConf` also isn't a strict merging, but now that it's getting more complicated, that would be ideal. So at least for this PR, if you could update your additions to be coming from their own function or functions, that would be helpful in working towards that goal.
If you want to wait until there's feedback from the more Flink-focused committers though (like openinx and Jingsong Li), then feel free. Wouldn't want to have my style request change be for nothing 🙂. But if these environment variables are commonly provided as you mentioned, I think that makes sense.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] hehuiyuan commented on pull request #3034: [ICEBERG-FLINK]support read hive configuration from HIVE_HOME&HIVE_CONF_DIR env
Posted by GitBox <gi...@apache.org>.
hehuiyuan commented on pull request #3034:
URL: https://github.com/apache/iceberg/pull/3034#issuecomment-906896926
Hi @JingsongLi @openinx , any suggestions for this issue？
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] hehuiyuan commented on pull request #3034: [ICEBERG-FLINK]support read hive configuration from HIVE_HOME&HIVE_CONF_DIR env
Posted by GitBox <gi...@apache.org>.
hehuiyuan commented on pull request #3034:
URL: https://github.com/apache/iceberg/pull/3034#issuecomment-908311617
Hi @kbendick , is that what you mean?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] hehuiyuan edited a comment on pull request #3034: [ICEBERG-FLINK]support read hive configuration from HIVE_HOME&HIVE_CONF_DIR env
Posted by GitBox <gi...@apache.org>.
hehuiyuan edited a comment on pull request #3034:
URL: https://github.com/apache/iceberg/pull/3034#issuecomment-906896926
Hi @JingsongLi @openinx , do you have any suggestions for this issue？
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] hehuiyuan commented on pull request #3034: [ICEBERG-FLINK]support read hive configuration from HIVE_HOME&HIVE_CONF_DIR env
Posted by GitBox <gi...@apache.org>.
hehuiyuan commented on pull request #3034:
URL: https://github.com/apache/iceberg/pull/3034#issuecomment-907583242
Hi @kbendick , thanks.
Update.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] hehuiyuan edited a comment on pull request #3034: [ICEBERG-FLINK]support read hive configuration from HIVE_HOME&HIVE_CONF_DIR env
Posted by GitBox <gi...@apache.org>.
hehuiyuan edited a comment on pull request #3034:
URL: https://github.com/apache/iceberg/pull/3034#issuecomment-908311617
Hi @kbendick , is that what you mean?
Take a look when you have time.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #3034: [ICEBERG-FLINK]support read hive configuration from HIVE_HOME&HIVE_CONF_DIR env
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #3034:
URL: https://github.com/apache/iceberg/pull/3034#discussion_r696983585
##########
File path: flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactory.java
##########
@@ -138,6 +139,21 @@ private static Configuration mergeHiveConf(Configuration hadoopConf, String hive
"There should be a hive-site.xml file under the directory %s", hiveConfDir);
newConf.addResource(new Path(hiveConfDir, "hive-site.xml"));
} else {
+ // If don't provide the hive-site.xml path explicitly, it will try to load resource from hive
+ // configuration with hive conf directory.
+ if(System.getenv("HIVE_HOME") != null){
+ File hiveSite = new File(System.getenv("HIVE_HOME") + "/conf", "hive-site.xml");
+ if(hiveSite.exists()){
Review comment:
Same nit about `if (`.
##########
File path: flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactory.java
##########
@@ -138,6 +139,21 @@ private static Configuration mergeHiveConf(Configuration hadoopConf, String hive
"There should be a hive-site.xml file under the directory %s", hiveConfDir);
newConf.addResource(new Path(hiveConfDir, "hive-site.xml"));
} else {
+ // If don't provide the hive-site.xml path explicitly, it will try to load resource from hive
+ // configuration with hive conf directory.
+ if(System.getenv("HIVE_HOME") != null){
Review comment:
Nit: Missing space after `if`
##########
File path: flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactory.java
##########
@@ -138,6 +139,21 @@ private static Configuration mergeHiveConf(Configuration hadoopConf, String hive
"There should be a hive-site.xml file under the directory %s", hiveConfDir);
newConf.addResource(new Path(hiveConfDir, "hive-site.xml"));
} else {
+ // If don't provide the hive-site.xml path explicitly, it will try to load resource from hive
+ // configuration with hive conf directory.
+ if(System.getenv("HIVE_HOME") != null){
+ File hiveSite = new File(System.getenv("HIVE_HOME") + "/conf", "hive-site.xml");
+ if(hiveSite.exists()){
+ newConf.addResource(new Path(hiveSite.getAbsolutePath()));
+ }
+ }
+ if(System.getenv("HIVE_CONF_DIR") != null){
Review comment:
Same nit about `if ()`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] hehuiyuan commented on pull request #3034: [ICEBERG-FLINK]support read hive configuration from HIVE_HOME&HIVE_CONF_DIR env
Posted by GitBox <gi...@apache.org>.
hehuiyuan commented on pull request #3034:
URL: https://github.com/apache/iceberg/pull/3034#issuecomment-915798243
Hi @JingsongLi @openinx , do you have any suggestions for this issue？
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] hehuiyuan commented on pull request #3034: [ICEBERG-FLINK]support read hive configuration from HIVE_HOME&HIVE_CONF_DIR env
Posted by GitBox <gi...@apache.org>.
hehuiyuan commented on pull request #3034:
URL: https://github.com/apache/iceberg/pull/3034#issuecomment-908311617
Hi @kbendick , is that what you mean?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org