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

Kafka versions #133

Closed
wants to merge 12 commits into from
Closed

Kafka versions #133

wants to merge 12 commits into from

Conversation

wizzat
Copy link
Collaborator

@wizzat wizzat commented Feb 25, 2014

This commit is a suggested fix for Issue #118, as well as other Kafka server version related issues. The approach here is to move the responsibility for knowing what Kafka server version is being used to the user and to provide reasonable and sane defaults for that particular Kafka server version.

Mark Roberts added 5 commits February 25, 2014 10:21
…o select Kafka version with appropriate defaults for core consumers and producers
…o select Kafka version with appropriate defaults for core consumers and producers
@rdiomar
Copy link
Collaborator

rdiomar commented Feb 26, 2014

The problem here is that we would have to add consumer classes for every kafka release. I prefer documenting which server versions are supported and handling any unrecognized api requests by raising an error that says "This API request was not recognized by the server"

@rdiomar
Copy link
Collaborator

rdiomar commented Feb 26, 2014

And of course setting the defaults to something that reduces errors and confusion

@mumrah
Copy link
Collaborator

mumrah commented Feb 26, 2014

Ideally, we can determine the Kafka server version and ignore certain features, like auto-commit. In the future there will be a Kafka API for getting the server version, but until then we have to simply fail with a good error message like @rdiomar suggest.

@wizzat
Copy link
Collaborator Author

wizzat commented Feb 26, 2014

There are two advantages to this approach:

  • This approach handles historic Kafka versions which do not support a API version.
  • This approach actually handles multiple versions of Kafka, and you're going to have to something like this no matter what, even if Kafka tells you what version it's on.
  • This approach simplifies the expected user API.

Also, failing with a good error message is not very "batteries included" when you can support multiple versions of Kafka seamlessly.

@wizzat
Copy link
Collaborator Author

wizzat commented Feb 26, 2014

"And of course setting the defaults to something that reduces errors and confusion"

I would argue that we should set the defaults to what should be considered "expected behavior". With the introduction of autocommit, it should be expected behavior that it be the default. Basically we are looking at a situation where the expected defaults will change over time in potentially non-backward compatible ways.

There are several ways to go about solving this problem:

  • Maintain a branch per Kafka server version and release multiple Kafka libraries to pypi. (import kafka80, import kafka81, ...)
  • Provide adapters which interpret the Kafka server version and provide appropriate defaults.
  • Handle failures and set defaults gracefully.
  • Provide safe defaults which do not provide expected behaviors.
  • Fail with intelligible error messages.

The approach represented in this patch is a variant on option 2, by requiring the user to specify the Kafka version (or use the latest). This approach seems the safest and most extensible for dealing with a live project with ongoing changes.

@wizzat wizzat closed this Sep 4, 2014
wbarnha added a commit to sibiryakov/kafka-python that referenced this pull request Mar 10, 2024
bradenneal1 pushed a commit to bradenneal1/kafka-python that referenced this pull request May 16, 2024
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.

3 participants