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

KafkaConsumer always passes message key to deserializer, even if key is None #913

Closed
jeffwidman opened this issue Dec 18, 2016 · 4 comments

Comments

@jeffwidman
Copy link
Collaborator

jeffwidman commented Dec 18, 2016

I specified json.loads as the KafkaConsumer's key_deserializer callable, then produced a message without a key.

json.loads throws an exception about expecting a string. Looks like root cause is here: https://github.com/dpkp/kafka-python/blob/master/kafka/consumer/fetcher.py#L546

Fetcher is always using passing the key to the deserializer, even if the key is None.

Will submit a PR shortly.

@jeffwidman jeffwidman changed the title KafkaConsumer always passes message key to deserializer, even if no key is null KafkaConsumer always passes message key to deserializer, even if key is None Dec 18, 2016
@dpkp
Copy link
Owner

dpkp commented Dec 19, 2016 via email

@jeffwidman
Copy link
Collaborator Author

jeffwidman commented Dec 19, 2016

Thanks for the fast reply.

I don't think we can require that all deserializers map None -> None

That makes sense.

We generally wrap json.loads in a lambda to check for None.

Good suggestion, hadn't thought of that. I'm guessing you mean lambda k: json.loads(k) if k is not None else k unless there's something special that I'm unaware of regarding how python's lambdas deal with None?

@jeffwidman
Copy link
Collaborator Author

jeffwidman commented Dec 19, 2016

Are you open to adding a note about this lambda tip to the tutorial doc, so that others don't have to debug it?

There's an example showing how to use the json.loads already, so just add another param on this line showing how to do this for keys with a comment like

 # using a lambda because null keys are expected, but json.loads throws exception if key=None

@jeffwidman jeffwidman reopened this Dec 19, 2016
@dpkp
Copy link
Owner

dpkp commented Dec 19, 2016

Yes -- exactly. Although I agree that it is much more common to want None -> None, and much less common to want something else, I dont think we should make it impossible to do the latter. Options I think are ok: improve docs, provide a "stock" json deserializer, add a configuration to disable None skipping.

Also you might want to take a quick look at #912 where I'm changing the internals of serializer / deserializer handling slightly.

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

No branches or pull requests

2 participants