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 2020/10/23 03:11:37 UTC
[GitHub] [iceberg] zhangminglei opened a new pull request #1651: add warehouse location check
zhangminglei opened a new pull request #1651:
URL: https://github.com/apache/iceberg/pull/1651
We should add check `hive.metastore.warehouse.dir` here, otherwise users must check the source code to solve.
----------------------------------------------------------------
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.
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] zhangminglei commented on pull request #1651: add warehouse location check
Posted by GitBox <gi...@apache.org>.
zhangminglei commented on pull request #1651:
URL: https://github.com/apache/iceberg/pull/1651#issuecomment-716363651
I think we can close 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.
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 change in pull request #1651: add warehouse location check
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1651:
URL: https://github.com/apache/iceberg/pull/1651#discussion_r511063697
##########
File path: hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java
##########
@@ -449,6 +449,10 @@ protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {
Database convertToDatabase(Namespace namespace, Map<String, String> meta) {
String warehouseLocation = conf.get("hive.metastore.warehouse.dir");
+ Preconditions.checkNotNull(
+ warehouseLocation,
+ "Warehouse location is not set: hive.metastore.warehouse.dir=null");
Review comment:
Style: indentation should be 4 spaces (2 indents) from the start of `Preconditions`.
----------------------------------------------------------------
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.
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] zhangminglei closed pull request #1651: add warehouse location check
Posted by GitBox <gi...@apache.org>.
zhangminglei closed pull request #1651:
URL: https://github.com/apache/iceberg/pull/1651
----------------------------------------------------------------
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.
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] zhangminglei removed a comment on pull request #1651: add warehouse location check
Posted by GitBox <gi...@apache.org>.
zhangminglei removed a comment on pull request #1651:
URL: https://github.com/apache/iceberg/pull/1651#issuecomment-716310724
> Looks good as a fix, but we may want to make `warehouseLocation` a catalog instance field so that we don't rely on this coming from the conf. We also want to support warehouse location in the catalog's 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.
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] zhangminglei commented on pull request #1651: add warehouse location check
Posted by GitBox <gi...@apache.org>.
zhangminglei commented on pull request #1651:
URL: https://github.com/apache/iceberg/pull/1651#issuecomment-716310724
> Looks good as a fix, but we may want to make `warehouseLocation` a catalog instance field so that we don't rely on this coming from the conf. We also want to support warehouse location in the catalog's 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.
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 #1651: add warehouse location check
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #1651:
URL: https://github.com/apache/iceberg/pull/1651#issuecomment-715504512
Looks good as a fix, but we may want to make `warehouseLocation` a catalog instance field so that we don't rely on this coming from the conf. We also want to support warehouse location in the catalog's 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.
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] zhangminglei closed pull request #1651: add warehouse location check
Posted by GitBox <gi...@apache.org>.
zhangminglei closed pull request #1651:
URL: https://github.com/apache/iceberg/pull/1651
----------------------------------------------------------------
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.
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] zhangminglei commented on pull request #1651: add warehouse location check
Posted by GitBox <gi...@apache.org>.
zhangminglei commented on pull request #1651:
URL: https://github.com/apache/iceberg/pull/1651#issuecomment-716311265
I think we already make `warehouseLocation` a catalog instance field which in https://github.com/apache/iceberg/blob/master/flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactory.java#L64
----------------------------------------------------------------
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.
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