-
Notifications
You must be signed in to change notification settings - Fork 383
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
[#3051] fix(kafka-catalog): Make the catalog creation failure exception more accurate #3060
Conversation
@@ -132,8 +134,16 @@ public void initialize(Map<String, String> config, CatalogInfo info) throws Runt | |||
AdminClientConfig.CLIENT_ID_CONFIG, | |||
String.format(CLIENT_ID_TEMPLATE, config.get(ID_KEY), info.namespace(), info.name())); | |||
|
|||
try { | |||
adminClient = AdminClient.create(adminClientConfig); |
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.
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.
Is it a UI problem or a backend problem?
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.
Backend. From the backend log, the list opearation block(retry) for a long time.
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.
It's because you are listing all the topics, the default API timeout is 1 minute, should I set it to a shorter time?
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.
I prefer to shorten the time here.
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.
Should we create a new issue to track this? Since is already a list topics operation, not a create catalog issue
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.
Should we create a new issue to track this? Since is already a list topics operation, not a create catalog issue
Agree, a separate issue may be better.
@yuqi1129 do you want to take another look? |
I have verified it, I'm okay with the changed. |
…xception more accurate (apache#3060) ### What changes were proposed in this pull request? - postpone default schema creation after admin client - reduced exception nesting ### Why are the changes needed? Fix: apache#3015 ### Does this PR introduce _any_ user-facing change? yes, when the Kafka catalog creation fails, use the correct error message ### How was this patch tested? IT added
What changes were proposed in this pull request?
Why are the changes needed?
Fix: #3015
Does this PR introduce any user-facing change?
yes, when the Kafka catalog creation fails, use the correct error message
How was this patch tested?
IT added