New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-19227][Table SQL / API] The catalog is still created after opening failed in catalog registering #13414
Conversation
The catalog is still created after opening failed in catalog registering
The catalog is still created after opening failed in catalog registering
The catalog is still created after opening failed in catalog registering
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit b392f6a (Fri Feb 19 07:23:00 UTC 2021) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution, left some comments.
@@ -223,6 +223,9 @@ public String getHiveVersion() { | |||
public void open() throws CatalogException { | |||
if (client == null) { | |||
client = HiveMetastoreClientFactory.create(hiveConf, hiveVersion); | |||
if (client == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok,i got it and i'll remove it
catalog.open(); | ||
catalogs.put(catalogName, catalog); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a case for this?
You can create a Catalog extends GenericInMemoryCatalog
and implement open
to throw exception. The check the TableEnv should not contains this catalog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your suggestions.i'll add a test case.
The catalog is still created after opening failed in catalog registering
@flinkbot run azure |
@flinkbot run azure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @zhuxiaoshang for the update, looks good to me! Merging...
…d in catalog registering This closes #13414
What is the purpose of the change
When I create the HiveCatalog and Flink is not able to connect to the HiveMetastore, the statement can not be executed, but the catalog is still created. Subsequent attempts to query the tables result in a NPE.
In CatalogManager.registerCatalog.
Consider open is a relatively easy operation to fail, we should put catalog into catalog manager after its open.
Brief change log
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation