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

Pass init options from MultiProcessConsumer down to SimpleConsumer #255

Closed
wants to merge 1 commit into from

Conversation

mahall
Copy link

@mahall mahall commented Oct 16, 2014

Fixes #248.

@@ -103,7 +107,8 @@ class MultiProcessConsumer(Consumer):
def __init__(self, client, group, topic, auto_commit=True,
auto_commit_every_n=AUTO_COMMIT_MSG_COUNT,
auto_commit_every_t=AUTO_COMMIT_INTERVAL,
num_procs=1, partitions_per_proc=0):
num_procs=1, partitions_per_proc=0,
simple_consumer_options={}):
Copy link
Collaborator

Choose a reason for hiding this comment

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

You... probably don't want to hard code an object in a function prototype. It's a source of endless bugs.

Copy link
Contributor

Choose a reason for hiding this comment

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

A mutable dict as a kwarg has import level sideeffects, it'll be one dict shared across all instances. Please assign this as None and then check inside the function.

@dpkp dpkp added the consumer label Jan 13, 2015
@sontek
Copy link
Contributor

sontek commented Feb 11, 2015

@mahall This doesn't merge cleanly. Can you get it working again?

@dpkp
Copy link
Owner

dpkp commented Mar 24, 2015

#336 improves on this a bit -- plan to merge that and close this unless there are strong objections.

@dpkp
Copy link
Owner

dpkp commented Mar 31, 2015

#336 merged -- thanks for your work on this issue!

@dpkp dpkp closed this Mar 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MultiProcessConsumer Cannot Set Options of SimpleConsumer Workers
4 participants