Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

(TWILL-265) Upgrading kafka version and removing lz4 dependency #84

Closed
wants to merge 1 commit into from

Conversation

CuriousVini
Copy link
Contributor

No description provided.

@CuriousVini CuriousVini force-pushed the TWILL-265-lz4-dep branch 9 times, most recently from 03f4dd6 to 67d9ffe Compare January 16, 2020 21:38
@@ -135,6 +135,9 @@ public void finished() {
server = new EmbeddedKafkaServer(kafkaServerConfig);
server.startAndWait();

// Wait a little while to make sure changes is reflected in broker service
TimeUnit.SECONDS.sleep(3);
Copy link
Member

Choose a reason for hiding this comment

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

It is probably worth to expose the BrokerService from the KafkaClient and you can have a more efficient wait by use the BrokerService.addChangeListener method or simply poll from the BrokerService.getBrokerList() method.

Copy link
Contributor Author

@CuriousVini CuriousVini Jan 17, 2020

Choose a reason for hiding this comment

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

I kept it same as testBrokerChange(). But I agree the suggested approach would be a little bit more efficient implementation.
I will open another pr with that enhancement to both the tests to keep the changes in this pr related to kafka version upgrade.

@asfgit asfgit closed this in 944fb79 Jan 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants