Skip to content

Minor Exception cleanup #1317

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

Merged
merged 1 commit into from
Dec 12, 2017
Merged

Minor Exception cleanup #1317

merged 1 commit into from
Dec 12, 2017

Conversation

jeffwidman
Copy link
Contributor

No description provided.

@jeffwidman jeffwidman requested a review from dpkp December 7, 2017 23:09
if tags is not None and not isinstance(tags, dict):
raise Exception('tags must be a dict if present.')
raise ValueError('tags must be a dict if present.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, both of these could be changed to KafkaConfigurationError

log.exception('StopIteration raised unpacking messageset: %s', e)
raise Exception('StopIteration raised unpacking messageset')
log.exception('StopIteration raised unpacking messageset')
raise RuntimeError('StopIteration raised unpacking messageset')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Potentially this should be a KafkaError although I'm unclear when to reach for that instead of RuntimeError

Copy link
Collaborator

@tvoinarovskyi tvoinarovskyi Dec 8, 2017

Choose a reason for hiding this comment

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

RuntimeError is what would be raised by the convention on Python3 if we have a not handled StopIteration raised inside of a generator, so seems fine to me

@jeffwidman jeffwidman force-pushed the minor-exception-cleanup branch from 96b9ca2 to 43511aa Compare December 8, 2017 02:45
@jeffwidman jeffwidman merged commit 580520b into master Dec 12, 2017
@jeffwidman jeffwidman deleted the minor-exception-cleanup branch December 12, 2017 19:06
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