Skip to content

Added a way to create topics and brokers by passing a custom configuration #15

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

Merged
merged 2 commits into from
May 17, 2016

Conversation

lucapertile
Copy link
Contributor

Cleaned up the formatting noise and implemented the suggested changes. This PR adds support for custom topic and broker creation by enabling clients to pass a custom config when they create a broker or a topic.

@@ -239,4 +242,12 @@ sealed trait EmbeddedKafkaSupport {
broker.startup()
broker
}

def createCustomTopic(topic: String, topicConfig: Properties)(implicit config: EmbeddedKafkaConfig): Unit = {
val zkSessionTimeoutMs = 10000
Copy link
Owner

Choose a reason for hiding this comment

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

maybe this and following vals should be pulled out of this method, at the trait level, in order to avoid instantation every time the method gets called

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I've moved them at the trait level as suggested.

@manub
Copy link
Owner

manub commented May 16, 2016

LGTM, just a minor comment.

@manub manub merged commit 4b435d2 into manub:master May 17, 2016
@manub
Copy link
Owner

manub commented May 17, 2016

Thanks - I'll be doing a release by the end of the week, 0.5.1.

@manub
Copy link
Owner

manub commented May 17, 2016

@lucapertile actually it slipped out of my mind that we don't have scaladoc nor readme for this. Could you please create another PR before releasing?

@@ -239,4 +246,8 @@ sealed trait EmbeddedKafkaSupport {
broker.startup()
broker
}

def createCustomTopic(topic: String, topicConfig: Properties)(implicit config: EmbeddedKafkaConfig): Unit = {
Copy link

@chris-zen chris-zen May 18, 2016

Choose a reason for hiding this comment

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

Working with Properties in Scala is a bit ugly, have you considered using a Map[String, String] for the interface of this function instead ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants