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

Copy TestUtils#createBrokerConfig to avoid depending on upstream internal API #1341

Merged
merged 6 commits into from
Feb 4, 2025
Merged

Copy TestUtils#createBrokerConfig to avoid depending on upstream internal API #1341

merged 6 commits into from
Feb 4, 2025

Conversation

trnguyencflt
Copy link
Member

@trnguyencflt trnguyencflt commented Jan 10, 2025 •
edited
Loading

This PR copies createBrokerConfig to kafka-rest so that we don't depend on internal API that might change frequently and can break build.

@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

@trnguyencflt trnguyencflt changed the title Copy createBrokerConfig from AK to kafka rest to avoid depending on u… Copy createBrokerConfig to avoid depending on upstream internal API Feb 4, 2025
@trnguyencflt trnguyencflt marked this pull request as ready for review February 4, 2025 15:06
@trnguyencflt trnguyencflt requested review from a team as code owners February 4, 2025 15:06
@airlock-confluentinc airlock-confluentinc bot force-pushed the KNET-17181-remove-testutils-createbrokerconfig-dep branch from 10ccd7c to 5664f6e Compare February 4, 2025 15:06
@trnguyencflt trnguyencflt changed the title Copy createBrokerConfig to avoid depending on upstream internal API Copy TestUtils#createBrokerConfigs to avoid depending on upstream internal API Feb 4, 2025
msn-tldr
msn-tldr approved these changes Feb 4, 2025
View reviewed changes
Copy link
Member

@msn-tldr msn-tldr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm, 1 comment above. Thanks for fixing.

kafka-rest/src/test/java/io/confluent/kafkarest/integration/ClusterTestHarness.java Show resolved Hide resolved
@trnguyencflt trnguyencflt changed the title Copy TestUtils#createBrokerConfigs to avoid depending on upstream internal API Copy TestUtils#createBrokerConfig to avoid depending on upstream internal API Feb 4, 2025
shubh-ranade
shubh-ranade approved these changes Feb 4, 2025
View reviewed changes
Copy link
Contributor

@shubh-ranade shubh-ranade left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@trnguyencflt trnguyencflt merged commit 4f9f47a into master Feb 4, 2025
2 checks passed
@trnguyencflt trnguyencflt deleted the KNET-17181-remove-testutils-createbrokerconfig-dep branch February 4, 2025 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msn-tldr msn-tldr msn-tldr approved these changes

@shubh-ranade shubh-ranade shubh-ranade approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@trnguyencflt @shubh-ranade @msn-tldr