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

Make producers take a topic argument at send rather than init time #111

Merged
merged 2 commits into from
Jan 31, 2014

Conversation

rdiomar
Copy link
Collaborator

@rdiomar rdiomar commented Jan 24, 2014

This allows a single producer to be used to send to multiple topics. See #110

Note: This is not backwards compatible

This allows a single producer to be used to send to multiple topics.
See dpkp#110
@rdiomar rdiomar mentioned this pull request Jan 24, 2014
@dpkp
Copy link
Owner

dpkp commented Jan 28, 2014

Instead of breaking backwards compatibility, why not keep topic as an optional param in Producer inits and use that in message calls as a fallback if no topic is specified?

@rdiomar
Copy link
Collaborator Author

rdiomar commented Jan 28, 2014

Since send_messages() takes a variable number of messages, it doesn't play well with optional params. If topic=None was just before *msgs, and we don't specify a topic when we call it, it will just get the value of the first message.

@dpkp
Copy link
Owner

dpkp commented Jan 31, 2014

I'm not a huge fan of the variadic *msgs interface, but you're right that we can't have topic be optional and maintain *msgs . Perhaps for future work it is worth considering something closer to the KeyedMessage interface used by the scala client.

Merging. This is not backwards compatible, but requiring a separate Producer instance for each topic is not scalable as discussed in issue #110.

dpkp added a commit that referenced this pull request Jan 31, 2014
Make producers take a topic argument at send rather than init time -- fixes Issue #110, but breaks backwards compatibility with previous Producer interface.
@dpkp dpkp merged commit 4abf7ee into dpkp:master Jan 31, 2014
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.

2 participants