asqui / psycopg-nestedtransactions Goto Github PK

A side-effect of:

with Transaction(cxn):
    pass

...is that after this block, cxn.commit and cxn.rollback are instance methods bound to cxn, which have a reference to cxn and therefore prevent it from becoming eligible for deterministic garbage collection when it falls out of scope.

This is because _restore_patches() does not correctly undo what _try_patch() does in all cases. Specifically in the case where cxn methods are already patched with an instance-level bound method, the behaviour is correct, but in the simple case where commit and rollack are only defined as functions on type(cxn), the net effect is that they get turned into bound methods on the instance.

In the case where they are not originally bound methods in the starting state, the correct way to restore this is:

del cxn.commit
del cxn.rollback

This will remove the bound methods from the instance dictionary, and allow the type-level functions to be accessible once again.

To guarantee correctness and predictability when introducing this to existing code, we could actively prevent setting cxn.autocommit or calling of cxn.commit() or cxn.rollback() to actively prevent manual transactions being used in conjunction with the context manager.

We could do this by monkey-patching these with an exception thrower while the connection is within the context manager.

Probably won't be too much documentation in the way of documentation, but it would be nice to publish it to ReadTheDocs as an exercise.

We could use Daniele's starter pack as a starting point and for inspiration and Sphinx reference: https://github.com/evildmp/daniele-procida-starter-docs

The README content should probably move in to these docs, and be replaced with a link to readthedocs.

I can see psycopg2 benefiting from this.

Did you make a PR to psycopg2?

Currently if a user does this:

cxn.autocommit = False
cur.execute('INSERT INTO table VALUES ('A'))
with Transaction(cxn):
    cur.execute('INSERT INTO table VALUES ('B'))
    raise Exception("uh oh")

Both inserts will be rolled back because the transaction begins on the first insert and our context manager doesn't start another one at the beginning.

The obvious fix is just commit when we enter into our managed transaction for the very first time from 'user' code (and not when entering into a nested transaction). However i can see this initial commit not being that expected for the user / a hidden gotcha?

We could just throw if someone starts one of our transactions with autocommit off already, but that feels even more horrible.

Using Coveralls? Or something else?

Again, this is probably a bit overkill, but it's a fun exercise to play around and get this infrastructure up and running, even for such a small repo.

Do we want to use Travis, or something else?

We should decide what matrix of versions we want to support. I suppose if we're aiming to have this included in psycopg.extras we'll need to at least cover the same matrix of Python and Postgres versions that psycopg supports?

We may also want a third axis for different psycopg versions while this is a separate library?

So we have two packages right now, nestedtransactions and pyscopg-nestedtransactions. What package structure do we actually want? Using a - in a package name in python is horrible so perhaps instead it should be a nested package like:

pyscopg.nestedtransactions

Also if the end goal is getting the code into pyscopg2 itself then I guess the package should be:

pyscopg2.nestedtransactions