Skip to content

Conversation

Kyle-Neale
Copy link
Contributor

What does this PR do?

Motivation

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Add the qa/skip-qa label if the PR doesn't need to be tested during QA.
  • If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

Copy link

codecov bot commented Sep 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.01%. Comparing base (87aab10) to head (a9b3669).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@AAraKKe AAraKKe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am guessing the error we are seeing now is because we need a newer version of the confluent-kafka package now that we are using the confluent images. We are supporting the same versions of kafka but maybe the interaction is different.

@@ -4,7 +4,7 @@
# If you bump the `confluent-kafka` version, also bump the `librdkafka` version in the `32_install_kerberos.sh` file
post-install-commands = [
"python -m pip uninstall -y confluent-kafka",
"python -m pip install --no-binary confluent-kafka confluent-kafka==2.8.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably need to bring a newer one now that we are using confluent kafka and dropped bitnami?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I assumed that they might have a python client with versions matching the confluent platform version we are using (as is commonly done), but looking at the client versions this is not the case. 2.11.1 seems to be the latest one so we should be good there. It was a wild guess about why tests could be failing.

@Kyle-Neale
Copy link
Contributor Author

I am guessing the error we are seeing now is because we need a newer version of the confluent-kafka package now that we are using the confluent images. We are supporting the same versions of kafka but maybe the interaction is different.

No, but the failure is expected since the bumped dependency is not yet built at this point so it's pulled the image from it's upstream repository. Once the dependency resolution gets merged we'll see these failures resolve itself.

@@ -0,0 +1 @@
Bump `confluent-kafka` to 2.11.1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor but still, please, add newline

@Kyle-Neale Kyle-Neale disabled auto-merge September 11, 2025 14:37
@Kyle-Neale Kyle-Neale merged commit 2aeb4bc into master Sep 11, 2025
55 of 58 checks passed
@Kyle-Neale Kyle-Neale deleted the kyle.neale/bump-kafka-dep branch September 11, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants