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.

Sign up for GitHub

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

Jump to bottom

Raising a FailedPayloadsError loses response information for other payloads #301

Closed
jobevers opened this issue Jan 22, 2015 · 4 comments
Closed

Raising a FailedPayloadsError loses response information for other payloads #301

jobevers opened this issue Jan 22, 2015 · 4 comments
Assignees
@dpkp
Milestone
0.9.4 Release

Comments

@jobevers
Copy link
Contributor

In client._send_broker_aware_request (https://github.com/mumrah/kafka-python/blob/master/kafka/client.py#L199), if failed_payloads exists, raising an Exception will cause the lose of the other response information (in acc)

Is there a reason for raising the Exception? Would it be acceptable to return that information along with the responses instead?
@dpkp
Copy link
Owner

dpkp commented Jan 22, 2015

agree -- the current error processing dates back to sept/oct 2013 and could use some love. have time to submit a PR?

@jobevers
Copy link
Contributor Author

It would be a backwards compatible change - I can try to take a stab at a PR if you're okay with that.

@dpkp
Copy link
Owner

dpkp commented Jan 22, 2015

please do - thanks!

@dpkp dpkp mentioned this issue Apr 6, 2015
Merged
@dpkp dpkp added this to the 0.9.4 Release milestone Apr 6, 2015
@dpkp dpkp self-assigned this Apr 6, 2015
@dpkp
Copy link
Owner

dpkp commented Apr 13, 2015

see #366 -- FailedPayloads() objects are now returned in the list of responses rather than raised immediately.

@dpkp dpkp closed this as completed Apr 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dpkp dpkp

Labels
None yet
Projects
None yet
Milestone
0.9.4 Release
Development

No branches or pull requests
2 participants
@dpkp @jobevers