Reset Session._flushing when a bulk_* call can't begin its transaction - #13487
Reset Session._flushing when a bulk_* call can't begin its transaction#13487hamodywe wants to merge 2 commits into
Conversation
_bulk_save_mappings() set self._flushing = True and called self._autobegin_t()._begin() before its try block, so an exception raised by _begin() -- such as PendingRollbackError when the transaction still needs a rollback from a prior failed flush -- left _flushing stuck at True. flush() does not have this problem: it sets the flag as the first line inside its try/finally. Neither Session.rollback() nor Session.close() reset the flag, so a reused Session stayed permanently unable to flush, always raising "Session is already flushing", until the object itself was discarded. Move the flag-set and transaction-begin inside a try/finally that wraps the whole method, matching flush()'s shape, so the flag is always cleared regardless of where the failure happens. Fixes: sqlalchemy#13485
| @@ -0,0 +1,15 @@ | |||
| .. change:: | |||
There was a problem hiding this comment.
let's move to 2.0, since this is something that has sense also in that relaease
rest of the code looks mostly good
There was a problem hiding this comment.
Moved to unreleased_20 as suggested — content unchanged
sqla-tester
left a comment
There was a problem hiding this comment.
OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 4b32839 of this pull request into gerrit so we can run tests and reviews and stuff
|
New Gerrit review created for change 4b32839: https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6882 |
The fix applies to 2.0 as much as to 2.1, so the note belongs in unreleased_20. Content unchanged.
|
hi i've taken this over, no further input needed thanks! |
thanx bro |
|
Michael Bayer (zzzeek) wrote: recheck View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6883 |
|
Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6882 has been merged. Congratulations! :) |
|
Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/6883 has been merged. Congratulations! :) |
Fixed bug where a failed ``Session.bulk_insert_mappings()``, ``Session.bulk_update_mappings()`` or ``Session.bulk_save_objects()`` call could leave the :class:`_orm.Session` permanently in a "flushing" state. ``Session._bulk_save_mappings()`` set ``self._flushing = True`` and called ``self._autobegin_t()._begin()`` before its ``try`` block, so an exception raised by ``_begin()`` -- such as ``PendingRollbackError`` when the transaction still needs a rollback from a prior failed flush -- left ``_flushing`` stuck at ``True``. As neither ``Session.rollback()`` nor ``Session.close()`` reset the flag, a reused Session then raised "Session is already flushing" for every subsequent flush. The flag-set and transaction-begin are moved inside a ``try``/``finally`` that wraps the whole method, matching the shape used by ``Session.flush()``, so that the flag is always cleared regardless of where the failure occurs. Fixes: #13485 Closes: #13487 Pull-request: #13487 Pull-request-sha: 4b32839 Change-Id: I5be8b59db500aacf2a76fe31f9c0c854a8324f70
_bulk_save_mappings() set self._flushing = True and called self._autobegin_t()._begin() before its try block, so an exception raised by _begin() -- such as PendingRollbackError when the transaction still needs a rollback from a prior failed flush -- left _flushing stuck at True. flush() does not have this problem: it sets the flag as the first line inside its try/finally.
Neither Session.rollback() nor Session.close() reset the flag, so a reused Session stayed permanently unable to flush, always raising "Session is already flushing", until the object itself was discarded.
Move the flag-set and transaction-begin inside a try/finally that wraps the whole method, matching flush()'s shape, so the flag is always cleared regardless of where the failure happens.
Fixes: #13485
Description
Checklist
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!