fix: @non_transactional decorator was not working correctly with async#554
Conversation
google-cla
Bot
added
the
cla: yes
| """ | ||
| transaction = _get_transaction(options) | ||
|
|
||
| transaction = options.transaction |
There was a problem hiding this comment.
So, what was going on was the transaction would be set or not for the batch when _datastore_api.lookup was called, but the call to _get_transaction would potentially fish the transaction out of the current context when the batch was actually run. The problem is that the call to lookup would be made in the context of a non_transactional and would correctly set the transaction to None, but then because the call to actually process the batch was delayed, it could happen outside of the non_transactional context, even though it is processing lookup calls made in the non_transactional context. The solution here is to just use the transaction set for the batch without referring to the current context. I'm pretty sure the call to _get_transaction is a fossil from a time before the transactions feature had been completely figured out. I have verified that in all cases the current transaction is set in options when the lookup call is made, so there really is no need to refer to the context.
cguardia
left a comment
cguardia
left a comment
There was a problem hiding this comment.
This looks good. The explanation clears it up for me.
In all cases, we know where to get the transaction from.
andrewsg
left a comment
andrewsg
left a comment
There was a problem hiding this comment.
Thanks, especially for the regression tests!
3 participants
Fixes #552