track cloned connections #167
Conversation
sqlalchemy_continuum/manager.py
Outdated
| @@ -378,6 +378,11 @@ def append_association_operation(self, conn, table_name, params, op): | |||
| uow = self.units_of_work[conn.engine] | |||
| uow.pending_statements.append(stmt) | |||
|
|
|||
| def track_cloned_connections(self, c, opt): | |||
| for connection, uow in self.units_of_work.items(): | |||
There was a problem hiding this comment.
if c not in self.units_of_work.keys() perhaps?
|
Right, now we need to get rid of those pesky clones in uow dict somehow ... |
netcriptus
force-pushed
the
hotfix/track-cloned-connections
branch
from
a560867 to
f0f6044
Compare
|
@kvesteri can we have your opinion on this? Looks about ready to me, tests are passing, and solves the issue I opened. It would be great to have this merged to master. |
|
Before merging this PR needs to:
|
netcriptus
force-pushed
the
hotfix/track-cloned-connections
branch
from
c2be980 to
ae0b16e
Compare
|
@kvesteri I had the same issue. Is this ready to be merged into master? |
|
This PR is missing a unit test. Otherwise this is merge-ready. |
|
+1 |
|
This PR is really useful to fix KeyError when trying to find the current "uow". +1 |
|
@netcriptus can you add a unit test that reproduces the original issue? |
Hotfix/track cloned connections (#2) Hotfix/track cloned connections (#3) change iterations use only ConnectionFairy as indexes create new dict entry for clones create entry using old connection create entry using session revert to event listening strategy remove cloned connections on rollback treat None case when cleaning connections Hotfix/track cloned connections (#2) Hotfix/track cloned connections (#3) change iterations use only ConnectionFairy as indexes create new dict entry for clones create entry using old connection create entry using session revert to event listening strategy remove cloned connections on rollback treat None case when cleaning connections
synchrone
force-pushed
the
hotfix/track-cloned-connections
branch
from
ae0b16e to
acef01b
Compare
|
@kvesteri alright, @synchrone and I added some tests that reproduce the bug if the patch is not there. One of the tests failed on the CI because of a network problem but I don't have permission to re-run it. Can this be merged now? |
This solves Issue 166 (with many thanks to @synchrone for debugging it!)