django-pgtrigger icon indicating copy to clipboard operation
django-pgtrigger copied to clipboard

Exiting `pgtrigger.ignore` while the transaction is in a failed state crashes

Open ralokt opened this issue 2 years ago • 1 comments

Code like this:


with pgtrigger.ignore(<some_trigger>):
    try:
         do_something() # raises database error
    except:
         # handle exception without reraising

Will currently crash, as the RESET statement emitted by pgtrigger will take place in a failed transaction.

This didn't use to be the case, and was most likely caused by the resolution of #49 .

While this isn't too common a use case, or one that should be encouraged - after all, it requires developers to disregard best practices around exception handling - tests are an exception to that rule. For example, it is currently impossible to write a test like so:


with pgtrigger.ignore(<some_trigger>):
    with pytest.raises(<database error>) as excinfo:
         do_something() # raises database error

# assert things about excinfo

Best workaround I could find is something like


with pgtrigger.ignore(<some_trigger>):
    with transaction.atomic():
        with pytest.raises(<database error>) as excinfo:
             do_something() # raises database error
        transaction.set_rollback(True)

# assert things about excinfo

Which is ugly, and nontrivial to arrive at.

ralokt avatar Sep 12 '22 15:09 ralokt

@ralokt yea it's expected behavior with a few notes on how to handle it here

You can use pgtrigger.ignore.session to start the ignore session earlier. For example, in pytest you can use a session fixture:

@pytest.fixture(scope="session", autouse=True)
def pgtrigger_ignore_session():
    with pgtrigger.ignore.session():
        yield

(Note that it can be used as a decorator too)

As you already mentioned in #49, one way around is this to always have the pre-execute hook registered, which is effectively what the above snippet does.

I could change the API so that you don't have to use pgtrigger.ignore.session as a context manager if that would help your case. I could also expose a setting so that it's always active. My main concern is that I've found a few examples here where SQL fails when prepending variables. I don't know how many other SELECT statements would be affected or what other issues it would cause.

Thoughts?

Going to mark this as a question for now. Will add the snippet to the docs about testing in the FAQ and might consider making testing a section in itself

wesleykendall avatar Sep 12 '22 16:09 wesleykendall