sync: rollbacks db on exception #4
Conversation
jrcastro2
force-pushed
the
add-rollback
branch
5 times, most recently
from
46d9b1d to
bf11b8f
Compare
invenio_cern_sync/users/sync.py
Outdated
|
|
||
| with db.session.begin_nested(): | ||
| _id = create_user(invenio_user) | ||
| db.session.flush() |
There was a problem hiding this comment.
This should not be needed? the begin_nested will commit the transaction on_exit
| db.session.flush() |
There was a problem hiding this comment.
Right, it should flush before entering and on exit, so we should be safe, thanks 👍
invenio_cern_sync/users/sync.py
Outdated
| db.session.flush() | ||
| inserted.add(_id) | ||
|
|
||
| db.session.commit() |
There was a problem hiding this comment.
each begin_nested should already commit each time
| db.session.commit() |
There was a problem hiding this comment.
Hmm, If I understand correctly the begin_nested does "commit" the SAVEPOINT meaning that it flushes those changes and does release the SAVEPOINT but doesn´t do the actual commit to the DB until we call db.session.commit()
When the context manager yielded by Session.begin_nested() completes, it “commits” the savepoint, which includes the usual behavior of flushing all pending state.
When the .commit() method on this object is called, “RELEASE SAVEPOINT” is emitted to the database, and if instead the .rollback() method is called, “ROLLBACK TO SAVEPOINT” is emitted. The enclosing database transaction remains in progress.
There was a problem hiding this comment.
You are right, it is only "closing" the transaction/savepoint.
However, we are now basically isolating the transaction, and potentially rollback. If one fails, the other will still work and succeed with a single db.session.commit() at the end.
I think we should find a tradeoff: what about having a commit after e.g. 1000 users?
jrcastro2
force-pushed
the
add-rollback
branch
from
bf11b8f to
ae88fd7
Compare
jrcastro2
force-pushed
the
add-rollback
branch
from
ae88fd7 to
536e69d
Compare
2 participants
No description provided.