Expose transaction cancellation reasons #1156
Conversation
This will allow the caller to know e.g. which of the transaction items ran into a conflict, which can sometimes allow app logic to better handle the error without needing to make more requests.
| CancellationReason( | ||
| code=d['Code'], | ||
| message=d.get('Message'), | ||
| ) if d['Code'] != 'None' else None |
There was a problem hiding this comment.
Why return None here? Shouldn't we instead not return anything at all?
The following code will break with AttributeError: 'NoneType' object has no attribute 'code':
except pydb_exc.TransactWriteError as e:
retry_err_types = (
'ProvisionedThroughputExceededException',
'ThrottlingError',
'TransactionConflict',
)
for error in e.cancellation_reasons:
if error.code in retry_err_types:
raise RuntimeError(f'transaction failed {error.code}')
Just seems like it'd be cleaner to not return anything vs returning None
There was a problem hiding this comment.
The cancellation reasons are 1:1 to the transaction items so you can know what item was the reason.
Would you rather the reason itself not to be none but for code to be none?
There was a problem hiding this comment.
Yeah to me that would feel cleaner but I guess it's subjective. Just negates the extra "if error and error.code"
There was a problem hiding this comment.
The thing is, if it was "cleaner", it would leave users with no way of associating reasons with individual transaction items. Maybe a different interface would convey this better, e.g. mapping of item to reason.
This will allow the caller to know e.g. which of the transaction items ran into a conflict, which can sometimes allow app logic to better handle the error without needing to make more requests.
This is a backport of #1144 to 5.x branch (adjusted to Python 3.6 compatibility, hence no dataclass).
Fixes #891.