Update consumer.py

[westover]

Just added a switch into the consumer so that no uncommenting is necessary for using kafka 0.8.1

[rdiomar]

This change requires the caller to know what version the server is running. I also don't think version-aware code is the way to go because then you'll have to modify it for every future version. Also, breaks travis.

[ghost]

Perhaps then a try except wrap because this does throw an exception if not available

[wizzat]

This problem needs some resolution, and I believe it's completely reasonable for the person making the call to know what version of Kafka the server is using. I've got a fork of Python-Kafka in production because I need to work with the full feature set of both 0.8.0 and 0.8.1.

[rdiomar]

It is reasonable for the caller to know the version, but we need to have a solution that does not involve hard-coding version numbers in the code because it's not scalable. If we used this approach for every new server version with an api change then it just won't be practical. The correct approach is probably to release a different version of kafka-python for each api version.

[wizzat]

The approach that I took was to put the kafka version in the same properties file which details its location. It works pretty well.

[rdiomar]

Even then, you still need to say

if version == 0.8.0:
  ...
elif version == 0.8.1:
  ...
elif version == 0.8.2:
  ...

Then in the future you'll have 20 versions that support different api's, and you'll need these if/else statements in a bunch of places.

[ghost]

Realistically we are only talking about differentiation between pre 0.8.1 and post 0.8.1 which is where this code becomes active

James Westover

On Mar 14, 2014, at 7:27 PM, Omar [email protected] wrote:

Even then, you still need to say
if version == 0.8.0:
...
elif version == 0.8.1:
...
elif version == 0.8.2:
...

Then in the future you'll have 20 versions that support different api's, and you'll need these if/else statements in a bunch of places.

—
Reply to this email directly or view it on GitHub.

[rdiomar]

That's true, I'm just questioning this approach in the general sense. It's almost guaranteed that there will be more api changes in future versions, and if we want to continue to support all versions at the same time then we will have these if/else's everywhere. I'm proposing releasing a version of kafka-python that supports 0.8.0, then once 0.8.1 is released we would update kafka-python, increment our version, and release that separately. Then the user can install the appropriate package based on which server they're running, and the code stays clean. This can be done for every server upgrade that has an api change.

[wizzat]

That doesn't seem very scalable from the pypi perspective. The approach I took was to make the user select a class representing their particular server version.

[rdiomar]

Other than 0.8.0-0.8.1, we will need to release a new version of kafka-python with every api change anyways, even with your approach. The difference is that we won't support older server versions and we would keep the codebase clean. If a user wants to run an older server they'll have to use an older version of kafka-python along with it. That seems reasonable to me. I'm pretty sure this is standard practice.

[wizzat]

This should be fixed in #158. I think we can close it?