HIVE-28658 Add Iceberg REST Catalog client support #5628
Conversation
Change-Id: I5bb2559f7ca602b71f8ca03c852e2deff1a1bc52
|
| Class.forName("org.apache.iceberg.hive.HiveIcebergRESTCatalogClientAdapter", true, Utilities.getSessionSpecifiedClassLoader()); | ||
| IMetaStoreClient restCatalogMetastoreClient = ReflectionUtils.newInstance(handlerClass, conf); | ||
| restCatalogMetastoreClient.reconnect(); | ||
| return restCatalogMetastoreClient; |
There was a problem hiding this comment.
I don't disagree with this approach. However, if we replace IMetaStoreClient, I'd like you to resolve HIVE-12679, which is more generic and allows users to use Glue or another service.
Committers have asked us to discover solutions to implement some of SessionHiveMetaStoreClient's traits.
#4444
That's why we should make configurable not IMetaStoreClient but ThriftHiveMetastore.Iface.
https://issues.apache.org/jira/browse/HIVE-28658?focusedCommentId=17905019&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17905019
There was a problem hiding this comment.
@okumin thanks for checking my PR. I see the point to make it configurable, but as I see the ticket you mentioned didn't get a conclusion and it was closed and not merged. So I would suggest to handle this separately and once your is merged we can refactor this. WDYT?
There was a problem hiding this comment.
@okumin could you please reopen your PR?
There was a problem hiding this comment.
My PR has not reached any conclusion with which maintainers can agree. We have to reconsidering how we integrate some session features such as tmp tables.
This is the summary and my prososal. I'd like to hear your opinions as my recommendation is not aligned your PR
https://docs.google.com/document/d/1fFvB0DAXJvPYv27R8nLa3tOUT67-hY0owxBrDY-BxUA/edit?usp=sharing
|
@zratkai Thanks for your PR! Could you give an example of a test? Or the records & screenshots you've tested? |
Change-Id: I5bb2559f7ca602b71f8ca03c852e2deff1a1bc52
What changes were proposed in this pull request?
Iceberg REST client implementation added to support Iceberg REST server connection.
Why are the changes needed?
To support Iceberg REST server connection.
Does this PR introduce any user-facing change?
No.
Is the change a dependency upgrade?
How was this patch tested?
Unit test.