Optionally include rollback exception in closed transaction invalid request error message when possible #11297
Conversation
…rich InvalidRequestError message
| trans_context = subject._trans_context_manager | ||
| if trans_context: | ||
| if not trans_context._transaction_is_active(): | ||
| transaction = getattr(subject, "transaction", None) |
There was a problem hiding this comment.
@zzzeek is using getattr in this context acceptable, or would you prefer that the _TConsSubject protocol be extended to also require subjects passed to _trans_ctx_check have a transaction attribute?
There was a problem hiding this comment.
the protocol should be expanded and I would stick to underscored names in all cases
| return self.nested or not self._parent | ||
|
|
||
| @property | ||
| def rollback_exception(self): |
There was a problem hiding this comment.
let's keep this underscore, dont want to add more public accessors, note TransactionalContext uses underscored method names as its API
| trans_context = subject._trans_context_manager | ||
| if trans_context: | ||
| if not trans_context._transaction_is_active(): | ||
| transaction = getattr(subject, "transaction", None) |
There was a problem hiding this comment.
the protocol should be expanded and I would stick to underscored names in all cases
| if trans_context: | ||
| if not trans_context._transaction_is_active(): | ||
| transaction = getattr(subject, "transaction", None) | ||
| if transaction is not None: |
| transaction = getattr(subject, "transaction", None) | ||
| if transaction is not None: | ||
| if transaction.rollback_exception is not None: | ||
| raise exc.InvalidRequestError( |
There was a problem hiding this comment.
these two raises that look identical should be just the one raise
|
there's changes requested on this PR if you're still around |
2 participants
Following discussion #11243
Description
SessionTransaction._rollback_exceptionusing public propertyrollback_exceptionTransactionalContext._trans_ctx_check()when a closed transaction is detected. first checking a transaction'srollback_exceptionis accessible and if it is, raising an InvalidRequestError which includes it in the error messageChecklist
This pull request is:
must include a complete example of the issue. one line code fixes without an
issue and demonstration will not be accepted.
Fixes: #<issue number>in the commit messageinclude a complete example of how the feature would look.
Fixes: #<issue number>in the commit messageHave a nice day!