django-pgtrigger
django-pgtrigger copied to clipboard
Exiting `pgtrigger.ignore` while the transaction is in a failed state crashes
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 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