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 2022/10/14 18:06:43 UTC
[GitHub] [iceberg] pavibhai opened a new pull request, #5989: Closes #5988 - Allow configuration of Hive MetastoreClient using Catalog properties
pavibhai opened a new pull request, #5989:
URL: https://github.com/apache/iceberg/pull/5989
## Why?
Currently Iceberg limits the Hive properties that can be configured onto a catalog to `uri -> hive.metastore.uris` and `warehouse -> hive.metastore.warehouse.dir`.
This limits the ability to configure the metastore client being set up. For example in a secure set up when using delegation tokens we need to configure different values for `hive.metastore.token.signature` for each metastore service.
We need a means for configuring each client differently on some properties and others are generic as derived from the classpath.
## What?
In this patch we allow any catalog properties that start with `hive.` to be passed on to the configuration used for creating the MetastoreClient.
## Testing?
Unit test was added to verify the above functionality and no regression failures.
--
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] rdblue commented on pull request #5989: Closes #5988 - Allow configuration of Hive MetastoreClient using Catalog properties
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5989:
URL: https://github.com/apache/iceberg/pull/5989#issuecomment-1285863344
@pavibhai, this looks reasonable to me. The reason why we haven't done this in the past is that we typically expect `hive-site.xml` to be used for deploying these settings. When that's the case, then we don't want to add more complexity. However, properties that may be specific to a catalog are good candidates for forwarding from catalog configuration.
--
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] pavibhai commented on a diff in pull request #5989: Closes #5988 - Allow configuration of Hive MetastoreClient using Catalog properties
Posted by GitBox <gi...@apache.org>.
pavibhai commented on code in PR #5989:
URL: https://github.com/apache/iceberg/pull/5989#discussion_r1000946314
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java:
##########
@@ -83,6 +84,15 @@ public void initialize(String inputName, Map<String, String> properties) {
this.conf = new Configuration();
}
+ // Forward all catalog properties that begin with `hive.` into configuration for use in setting
+ // up the
+ // MetastoreClient
Review Comment:
Will do
--
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] rdblue commented on pull request #5989: Closes #5988 - Allow configuration of Hive MetastoreClient using Catalog properties
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5989:
URL: https://github.com/apache/iceberg/pull/5989#issuecomment-1286295392
Thanks, @pavibhai!
--
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] pavibhai commented on pull request #5989: Closes #5988 - Allow configuration of Hive MetastoreClient using Catalog properties
Posted by GitBox <gi...@apache.org>.
pavibhai commented on PR #5989:
URL: https://github.com/apache/iceberg/pull/5989#issuecomment-1285936547
> Looks good to me. I'd just like to have the formatting fixed.
@rdblue I have addressed this, please take another look when you get a chance.
--
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] rdblue commented on a diff in pull request #5989: Closes #5988 - Allow configuration of Hive MetastoreClient using Catalog properties
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5989:
URL: https://github.com/apache/iceberg/pull/5989#discussion_r1000879572
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java:
##########
@@ -83,6 +84,15 @@ public void initialize(String inputName, Map<String, String> properties) {
this.conf = new Configuration();
}
+ // Forward all catalog properties that begin with `hive.` into configuration for use in setting
+ // up the
+ // MetastoreClient
Review Comment:
Looks like this was auto formatted and could be fixed.
--
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] rdblue merged pull request #5989: Closes #5988 - Allow configuration of Hive MetastoreClient using Catalog properties
Posted by GitBox <gi...@apache.org>.
rdblue merged PR #5989:
URL: https://github.com/apache/iceberg/pull/5989
--
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] pavibhai commented on pull request #5989: Closes #5988 - Allow configuration of Hive MetastoreClient using Catalog properties
Posted by GitBox <gi...@apache.org>.
pavibhai commented on PR #5989:
URL: https://github.com/apache/iceberg/pull/5989#issuecomment-1285934329
> When that's the case, then we don't want to add more complexity. However, properties that may be specific to a catalog are good candidates for forwarding from catalog configuration.
Thanks @rdblue for taking a look. This is exactly how we are using as well, all the common properties are derived from other means e.g. hive-site.xml but the specific properties are configured at the catalog level.
--
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