-
Notifications
You must be signed in to change notification settings - Fork 3k
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
tests(kafka-connect): fixes integration tests setup #12531
tests(kafka-connect): fixes integration tests setup #12531
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. see 26 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
It didn't solve all the problems but moved one step further
|
Most of the packages installed via
|
# | ||
confluent-hub install --no-prompt confluentinc/kafka-connect-s3:10.5.1 | ||
#confluent-hub install --no-prompt confluentinc/kafka-connect-s3:10.5.1 | ||
wget https://d2p6pa21dvn84.cloudfront.net/api/plugins/confluentinc/kafka-connect-s3/versions/10.5.1/confluentinc-kafka-connect-s3-10.5.1.zip |
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.
are these urls documented anywhere on confluent kafka docs?
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 looks brittle as per this - https://forum.confluent.io/t/the-cloudfront-we-used-for-libraries-doesnt-work-anymore/10992/6
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.
The cloudfront domain is documented here https://docs.confluent.io/platform/current/connect/confluent-hub/index.html#domain-allowlists
Actually, it changed in July 2024 https://support.confluent.io/hc/en-us/articles/28549788007060-Confluent-Hub-Connector-Domain-Change Not sure if that matches on time when flaky kafka tests started
Installing on using the zip file is documented here https://docs.confluent.io/platform/7.2/connect/confluent-hub/client.html#install-while-offline-using-a-zip-file
I found the suggestion on installing from the downloaded zips on some threads on internet where people was complaining about confluent-hub
cli not working consistently. I cannot find those threads again 😓
And the URL for the zip itself is got from https://www.confluent.io/hub/confluentinc/kafka-connect-jdbc when you download an specific version
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.
does update in commit 91ff618 look better? 😄
worked fine locally, let's see in GHA
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 we detect when the confluent-hub cli failed and retry the command?
Agree with Mayuri that the wget zip thing feels brittle
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.
confluent-hub cli retries was my first attempt to solve this, but this was consistently failing
anyway, in last commit I rely on the confluent cli instead: confluent connect plugin install
, so no wget zip anymore
kafka connect tests recurrently failing with
which is likely caused by
Validated this one
by running locally
and so the proposed fix in this PR
Checklist