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.

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

Support custom broker settings #12

Closed
Cs4r opened this issue May 14, 2018 · 5 comments
Closed

Support custom broker settings #12

Cs4r opened this issue May 14, 2018 · 5 comments
Assignees

Comments

@Cs4r
Copy link

Cs4r commented May 14, 2018

KafkaTestServer doesn't support any way of overriding the kafka broker settings.

e.g: num.io.threads, auto.create.topics.enable, port and zookeeper.session.timeout.ms are all hardcoded in KafkaTestServer::start.

@Crim
Copy link
Collaborator

Crim commented May 15, 2018

Hi @Cs4r thanks for the suggestion!

I'd love to understand/talk through how exactly you'd like to see this work. I could see a constructor argument being added to KafkaTestServer that would allow for passing in broker settings. That seems like a super easy thing to add.

I'm curious how you would see that being used by the Kafka-JUnit4 ClassRule? Would you just pass it on the constructor for SharedKafkaTestResource?

IE:

@ClassRule
    public static final SharedKafkaTestResource sharedKafkaTestResource = new SharedKafkaTestResource(customBrokerProperties);

How about for the JUnit5 variant? Perhaps something similar making use of the @RegisterExtension annotation instead of the @ExtendWith annotation?

@Crim Crim self-assigned this May 15, 2018
@Cs4r
Copy link
Author

Cs4r commented May 15, 2018

Hi @Crim,

Thanks for your quick response.

For me it would be very useful to tune some of the broker settings in order to proof that my custom consumer/producer behaves correctly in certain particular scenarios. e.g: what would happen if it tries to send a message to a topic that doesn't exist? That's not currently possible since auto.create.topics.enable is set to true.

I also thought of adding an argument constructor to KafkaTestServer , such constructor would take a java.util.Properties containing only the properties you want to override. Your suggestions about the integration with JUnit4 and JUnit5 seen sensible to me.

It doesn't seem to be very complicated unless I am missing something.

Regards

@Crim
Copy link
Collaborator

Crim commented May 16, 2018

@Cs4r Can you please review the READMEs to verify this new behavior will satisfy your needs?

Kafka-JUnit4 README

Kafka-JUnit5 README

@Cs4r
Copy link
Author

Cs4r commented May 16, 2018

Thank you for being so quick addressing the issue, I appreciate it.

Crim added a commit that referenced this issue May 17, 2018
for #12 

- [Issue-12](#12) Added ability to pass broker properties to be used by test kafka service instance.
- Added helper method getAdminClient() on KafkaTestServer to get a configured AdminClient instance.
- Deprecated Kafka-JUnit5 @ExtendWith annotation implementations.  This has been replaced in favor of @RegisterExtension annotation.  Review [README.md](kafka-junit5/README.md) for more information on updated usage instructions.
- Deprecated KafkaTestServer constructor: `public KafkaTestServer(final String localHostname)`
  
  This constructor was replaced with the constructor `KafkaTestServer(final Properties overrideBrokerProperties)` where overrideBrokerProperties should contain the property `host.name` set to the hostname or IP address Kafka should use.
@Crim
Copy link
Collaborator

Crim commented May 17, 2018

I just published 2.3.0 to the maven repository. If you run into any troubles or have any other great ideas feel free to submit a new issue.

Thanks!

@Crim Crim closed this as completed May 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants