-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add retry information to ResponseMetadata #1024
Conversation
Error messages from ClientErrors will now look like: An error occurred (C) when calling the O operation (max_retry=4 reached): M If the error response does not have retry information the client error is unchanged.
Current coverage is 97.57% (diff: 100%)@@ develop #1024 diff @@
==========================================
Files 44 44
Lines 6867 6884 +17
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 6700 6717 +17
Misses 167 167
Partials 0 0
|
@@ -342,17 +342,30 @@ class UnsupportedSignatureVersionError(BotoCoreError): | |||
class ClientError(Exception): | |||
MSG_TEMPLATE = ( | |||
'An error occurred ({error_code}) when calling the {operation_name} ' | |||
'operation: {error_message}') | |||
'operation{retry_info}: {error_message}') |
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.
Sort of unfortunate that retry_info
has to remember to put spaces, but I do not think there is any way around it if we want to put it right between operation and the colon.
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.
Yeah. The first version I had rendered the msg, but before calling super()
would mess with the msg to inject retry info. This approach seemed less worse that the first approach.
Also open to other ways to improve this.
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.
Makes sense. I am fine with how it is now. I cannot think of any other ways.
Looks good. Just had small questions and comments. |
@kyleknap updated. |
Thanks! 🚢 assuming the tests pass. |
This pulls in #965 with the following changes:
Retries
list with the number of retry attempts.ClientError
messages to include retry information when max attempts are reached.More context here.
A few examples:
Client error messages:
ResponseMetadata on error:
ResponseMetadata on success:
cc @kyleknap @JordonPhillips