Skip to content

Commit

Permalink
Commit instead of flushing in unit test
Browse files Browse the repository at this point in the history
Rationale:
We use an in-memory sqlite database for quota tests. With SA 2.0 (or
with the `future` flag enabled on the engine), the following conflict
happens: (this is a simplified model)

foo = Foo()
session.add(foo)
session.flush()

engine = session.get_bind()
with engine.connect() as conn:
    conn.execute(some-sql)

foo.bar = "new value"
session.commit() # BOOM!!!!  sqlalchemy.orm.exc.StaleDataError: UPDATE statement on table 'galaxy_user' expected to update 1 row(s); 0 were matched.

Reason for BOOM:
With an in-memory database, the underlying dbapi_connection object is
the same for the session and the engine.connect(). Here's what happens:

line 10: foo is flushed to the db tmp buffer
line 14: conn is closed on exit from context manager, which issues a
rollback, which rolls back whatever is in the tmp buffer - so foo is
never inserted.
line 16: foo is updated
line 17: error happens: the session thinks it's updating foo's record in
the db, but that record does not exist therefore, "0 rows matched".

Solution: commit instead of flushing - then foo is inserted.
  • Loading branch information
jdavcs committed Dec 12, 2023
1 parent 475f722 commit 377f783
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions test/unit/data/test_galaxy_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ def setUpClass(cls):
@classmethod
def persist(cls, *args, **kwargs):
session = cls.session()
flush = kwargs.get("flush", True)
commit = kwargs.get("commit", True)
for arg in args:
session.add(arg)
if flush:
session.flush()
if kwargs.get("expunge", not flush):
if commit:
session.commit()
if kwargs.get("expunge", not commit):
cls.expunge()
return arg # Return last or only arg.

Expand Down Expand Up @@ -255,7 +255,7 @@ def test_collection_get_interface(self):
model.DatasetCollectionElement(collection=c1, element=d1, element_identifier=f"{i}", element_index=i)
for i in range(elements)
]
self.persist(u, h1, d1, c1, *dces, flush=False, expunge=False)
self.persist(u, h1, d1, c1, *dces, commit=False, expunge=False)
self.model.session.flush()
for i in range(elements):
assert c1[i] == dces[i]
Expand Down

0 comments on commit 377f783

Please sign in to comment.