-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Fail Fast if Resources Do Not Exist in Kafka Cluster. #34659
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
base: master
Are you sure you want to change the base?
Conversation
4da28e6
to
5f038ad
Compare
Assigning reviewers. If you would like to opt out of this review, comment R: @m-trieu for label java. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
LGTM |
R: @johnjcasey for final approval |
6313f99
to
0f1f8f1
Compare
sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaIO.java
Show resolved
Hide resolved
consumer.partitionsFor(providedTopic).stream() | ||
.map(PartitionInfo::partition) | ||
.collect(Collectors.toSet()); | ||
if (spec.getLogTopicVerification() == null || !spec.getLogTopicVerification()) { |
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.
we could simplify these checks by defaulting the value to false
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.
Leaving as a TODO for now as mentioned offline. If its not super important we can merge as is, otherwise I'll revisit this next week.
0f1f8f1
to
321e52d
Compare
321e52d
to
19a58c0
Compare
This PR is a fix and reintroduction of #34258.
ORIGINAL DESCRIPTION: If a specified topic or partition do not exist in a kafka cluster, the pipeline should fail before submission. There are some caveats to this, such as whether or not the cluster is reachable, so we only fail in the case where the cluster is reachable but the specified partitions or topics are not present.
This PR also adds a new change to address #34630 which details an error if the local machine cannot connect to the cluster during verification steps. The error is no longer thrown and a new test case has been added to verify this issue no longer happens.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123
), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>
instead.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.