Skip to content

Conversation

@kecaps
Copy link
Contributor

@kecaps kecaps commented Mar 31, 2015

I noticed that python3 string topics were supported in SimpleProducer.send_messages but not elsewhere in the API. This change makes python3 string topics supported throughout the public API.

This required changes in some testing code to ensure that bytes were used when building internal structs for return and sending to kafka service.

Copy link
Owner

Choose a reason for hiding this comment

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

why not simply set topic = kafka_bytestring(topic) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did it this way as a premature optimization because I saw has_metadata_for_topic and get_partition_ids_for_topic were on the callpath of send_messages which is probably the most used method and always calls these metadata methods with a byte string. But on closer inspection, it should only call them once on initialization, so its probably clearer to use a consistent pattern.

@dpkp
Copy link
Owner

dpkp commented Apr 3, 2015

thanks -- test error looks like a travis-ci failure. do you mind squashing your second commit or git commit --amend ?

dpkp added a commit that referenced this pull request Apr 4, 2015
Make external API consistently support python3 strings for topic.
@dpkp dpkp merged commit 45c05d0 into dpkp:master Apr 4, 2015
@dpkp
Copy link
Owner

dpkp commented Apr 4, 2015

thanks!

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