-
Notifications
You must be signed in to change notification settings - Fork 19
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
Optionally propagate exceptions during message sending #18
Conversation
Didn't realize that would fail on 2.7. I'll update shortly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! Requesting a few changes. I haven't looked at the test yet, but will shortly.
@benhoyt Thanks for the feedback! Just pushed the changes. Not sure if I need to mark the requested changes resolved or not. First time doing a RP in someone's repo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks! Just one more change required to test_sender_disable_raise_in_async
.
@@ -42,6 +42,7 @@ def __init__(self, host, port=2003, prefix=None, timeout=5, interval=None, | |||
Use "tags" to specify common or default tags for this Sender, which | |||
are sent with each metric along with any tags passed to send(). | |||
""" | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't worry for now, but it's best to avoid extraneous formatting in other repos, unless you're changing it to match the existing style in that repo. People have different styles and it just adds noise to the PR (and potential for bikeshedding).
Merged, thank you! |
Option added to raise exceptions during message sending.