Skip to content
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.

Sign up for GitHub

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

Jump to bottom

dt/deps: Use setuptools for confluent-kafka source build #16283

Merged
merged 1 commit into from
Jan 26, 2024
Merged

dt/deps: Use setuptools for confluent-kafka source build #16283

merged 1 commit into from
Jan 26, 2024

Conversation

oleiman
Copy link
Member

@oleiman oleiman commented Jan 25, 2024 •
edited

e09c68a moved installation of the confluent-kafka package to the ducktape Dockerfile to avoid an unfortunate config in the prebuilt wheels from PyPi.

Apparently, --no-binary works equally well to specify individual
dependencies in a normal setuptools build. This has the added benefit
that the dependency is picked up by dev environments outside docker.

Fixes https://github.com/redpanda-data/core-internal/issues/1019

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x
  • v23.1.x

Release Notes

  • none

@oleiman oleiman self-assigned this Jan 25, 2024
@oleiman oleiman marked this pull request as ready for review January 25, 2024 00:50
@oleiman oleiman requested a review from a team as a code owner January 25, 2024 00:50
@oleiman oleiman requested review from savex, travisdowns and a team and removed request for a team January 25, 2024 00:50
@oleiman
Copy link
Member Author

oleiman commented Jan 25, 2024

/dt

@oleiman oleiman marked this pull request as draft January 25, 2024 03:45
@oleiman
Copy link
Member Author

oleiman commented Jan 25, 2024

/ci-repeat 1
debug
skip-units
tests/rptest/tests/redpanda_kerberos_test.py::GSSAPIReauthTest

@oleiman
Copy link
Member Author

oleiman commented Jan 25, 2024

/ci-repeat 1
debug
skip-redpanda-build
skip-units
tests/rptest/tests/redpanda_kerberos_test.py::GSSAPIReauthTest

@oleiman oleiman marked this pull request as ready for review January 25, 2024 18:31
@oleiman
Copy link
Member Author

oleiman commented Jan 25, 2024

/dt

@vbotbuildovich
Copy link
Collaborator

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/44304#018d422b-6319-4c9a-b594-72be7a199af4

travisdowns
travisdowns reviewed Jan 26, 2024
View reviewed changes
tests/docker/Dockerfile Outdated
python3 -m pip install --force --no-cache-dir -e /root/tests/

RUN python3 -m pip install --force-reinstall --no-cache-dir --no-binary :all: confluent-kafka==2.2.0
python3 -m pip install --force-reinstall --no-cache-dir --no-binary confluent-kafka -e /root/tests/
Copy link
Member

Choose a reason for hiding this comment

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

So this will pick up the right version from the setup.py, right? Or will it always install the current version?

Copy link
Member Author

Choose a reason for hiding this comment

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

As I understand it, the "confluent-kafka" there in the command line is strictly an argument to the no-binary option, then the actual package specification is read from root/tests/setup.py as before.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, thanks, I had misunderstood was going on here the first time around in part due to the diff. Now that I get it, it looks even better.

travisdowns
travisdowns reviewed Jan 26, 2024
View reviewed changes
tests/docker/Dockerfile
@@ -248,9 +248,7 @@ RUN python3 -m pip install --force --no-cache-dir -r /opt/scripts/consumer_offs
# passes --force so system pip packages can be updated
COPY --chown=0:0 --chmod=0755 tests/setup.py /root/tests/
RUN python3 -m pip install --upgrade --force pip && \
Copy link
Member

Choose a reason for hiding this comment

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

I think we should add a comment on what's going on here with confluetn kafka, perhaps just linking back to the issue this solves.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, fine by me 👍

travisdowns
travisdowns reviewed Jan 26, 2024
View reviewed changes
Copy link
Member

@travisdowns travisdowns left a comment

Choose a reason for hiding this comment

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

A couple of questoins.

@oleiman
dt/deps: Return confluent-kafka to tests/setup.py
f2e687a 
e09c68a moved installation of the confluent-kafka package to the
ducktape Dockerfile to avoid an unfortunate config in the prebuilt
wheels from PyPi.

Apparently, `--no-binary` works equally well to specify individual
dependencies in a normal setuptools build. This has the added benefit
that the dependency is picked up by dev environments outside docker.

Fixes redpanda-data/core-internal#1019
@oleiman
Copy link
Member Author

oleiman commented Jan 26, 2024

force push:

  • add a comment
  • revert --force-reinstall to --force (accidental change)

@travisdowns travisdowns self-requested a review January 26, 2024 16:17
travisdowns
travisdowns approved these changes Jan 26, 2024
View reviewed changes
Copy link
Member

@travisdowns travisdowns left a comment

Choose a reason for hiding this comment

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

LGTM and much thanks!

@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Jan 26, 2024 •
edited

new failures in https://buildkite.com/redpanda/redpanda/builds/44355#018d46ba-2f3c-4d67-9552-d3021c0a7214:

"rptest.tests.prefix_truncate_recovery_test.PrefixTruncateRecoveryTest.test_prefix_truncate_recovery.acks=-1.start_empty=False"

new failures in https://buildkite.com/redpanda/redpanda/builds/44355#018d46cb-8b2d-41c8-9c4c-1b0c881946dc:

"rptest.tests.cloud_storage_timing_stress_test.CloudStorageTimingStressTest.test_cloud_storage.cleanup_policy=delete"

new failures in https://buildkite.com/redpanda/redpanda/builds/44355#018d46cb-8b27-4ec3-9f3e-663280271d8a:

"rptest.tests.cloud_storage_timing_stress_test.CloudStorageTimingStressTest.test_cloud_storage_with_partition_moves.cleanup_policy=delete"

@oleiman
Copy link
Member Author

oleiman commented Jan 26, 2024 •
edited

CI Failures:

@rockwotj rockwotj merged commit 2fea9cc into redpanda-data:dev Jan 26, 2024
13 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@travisdowns travisdowns travisdowns approved these changes

@savex savex Awaiting requested review from savex savex is a code owner automatically assigned from redpanda-data/devprod

Assignees

@oleiman oleiman

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@oleiman @vbotbuildovich @travisdowns @rockwotj