Allow injecting clients into Kafka Consumers and Producers. #735
Conversation
kafka/consumer/group.py
Outdated
| @@ -201,6 +204,8 @@ class KafkaConsumer(six.Iterator): | |||
| } | |||
|
|
|||
| def __init__(self, *topics, **configs): | |||
| client = configs.pop("client", None) | |||
There was a problem hiding this comment.
The pattern we've been using so far is to put this in the class-level DEFAULT_CONFIG dict. Just add an entry for 'client': None
|
Just a few comments inline re: how to set default config values |
|
Moved. Thanks. |
|
Thanks -- I'll wait for tests to pass and then merge. |
kafka/producer/kafka.py
Outdated
| @@ -270,7 +274,7 @@ def __init__(self, **configs): | |||
| if self.config['acks'] == 'all': | |||
| self.config['acks'] = -1 | |||
|
|
|||
| client = KafkaClient(**self.config) | |||
| self.client = self.config['client'] or KafkaClient(**self.config) | |||
There was a problem hiding this comment.
this should just be client = not self.client
|
tests are failing |
|
@Julian I know this is super late, but are you still interested in updating this or should we close it? I think it's a good idea, although if your rationale is injecting fake clients purely for testing purposes, the work of #1032 / #1230 (rationale in #560) may make your life easier for testing the protocol layer separate from the network layer. |
|
@jeffwidman that looks promising! It looks like I've already seen #560 before (we currently use afkak for the Twisted case) so definite +1 from me. Afkak's internal interfaces are very clearly super close to what's in here, so being able to separate the protocol parts separately definitely sounds like it'd help. Given that we've never been able to upgrade to the newer kafka-python versions/clients because of this issue (and have stuck with the older versions) I suspect it's still relevant for us, but will have to spend a few minutes reviewing the patch here and reviewing our Kafka fake (which I'd also love to upstream :). I suspect I would still like to get this merged if possible, so assuming that's the case, I'll have a look and see where this is. |
|
Apologies, our test suite was broken due to upstream dependency changes in pylint and lz4. I've pinned the versions in master. Do you mind doing a rebase or merge and push for a new test run? |
|
@dpkp done |
|
Afraid tests are failing... |
Merge latest official repo
We use the older interfaces and are considering evaluating the newer ones, but in order to do any testing, we need to be able to inject fake clients into consumers and producers to prevent them from doing networking.
It would also be nice to prevent some of the other side effects that happen in init on these newer objects, but that work isn't done yet here.
Love to hear your thoughts obviously. Cheers.
This change is