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

Stop using broker-errors for client-side problems #1639

Merged
merged 1 commit into from
Nov 18, 2018

Conversation

jeffwidman
Copy link
Collaborator

@jeffwidman jeffwidman commented Nov 17, 2018

UnsupportedVersionError is intended to indicate a server-side error:

class UnsupportedVersionError(BrokerResponseError):
errno = 35
message = 'UNSUPPORTED_VERSION'
description = 'The version of API is not supported.'

So we should not be raising it for client-side errors. I realize that
semantically this seems like the appropriate error to raise. However,
this is confusing when debugging... for a real-life example, see
Parsely/pykafka#697. So I strongly feel that
server-side errors should be kept separate from client-side errors,
even if all the client is doing is proactively protecting against
hitting a situation where the broker would return this error.

Fix #1627


This change is Reviewable

@jeffwidman jeffwidman force-pushed the dont-use-broker-errors-for-client-side-problems branch 2 times, most recently from 7da8eb1 to dd8d304 Compare November 17, 2018 10:10
`UnsupportedVersionError` is intended to indicate a server-side error:
https://github.com/dpkp/kafka-python/blob/ba7372e44ffa1ee49fb4d5efbd67534393e944db/kafka/errors.py#L375-L378

So we should not be raising it for client-side errors. I realize that
semantically this seems like the appropriate error to raise. However,
this is confusing when debugging... for a real-life example, see
Parsely/pykafka#697. So I strongly feel that
server-side errors should be kept separate from client-side errors,
even if all the client is doing is proactively protecting against
hitting a situation where the broker would return this error.
@jeffwidman jeffwidman force-pushed the dont-use-broker-errors-for-client-side-problems branch from dd8d304 to aad278f Compare November 18, 2018 00:23
@jeffwidman jeffwidman merged commit f3105a4 into master Nov 18, 2018
@jeffwidman jeffwidman deleted the dont-use-broker-errors-for-client-side-problems branch November 18, 2018 08:20
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.

1 participant