Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't emit logs before rollback() #3125

Merged

Conversation

portante
Copy link
Member

When handling exceptions, certain kinds of exceptions in either SQLAlchemy or in PyscoPG2 will cause references to the SQLAlchemy object instance to cause other exceptions. For example, one might see:

sqlalchemy.exc.PendingRollbackError: This Session's transaction has
been rolled back due to a previous exception during flush. To begin
a new transaction with this Session, first issue Session.rollback().

By moving the log messages after the rollback() call, we avoid the extra mess caused.

We also use the self.logger where possible, and we enhance the existing state transition error log with the object information, and add a debug log message on the success path.

When handling exceptions, certain kinds of exceptions in either
SQLAlchemy or in PyscoPG2 will cause references to the SQLAlchemy object
instance to cause other exceptions.  For example, one might see:

    sqlalchemy.exc.PendingRollbackError: This Session's transaction has
    been rolled back due to a previous exception during flush. To begin
    a new transaction with this Session, first issue Session.rollback().

By moving the log messages after the `rollback()` call, we avoid the
extra mess caused.

We also use the `self.logger` where possible, and we enhance the
existing state transition error log with the object information, and
add a debug log message on the success path.
@portante portante added this to the v0.72 milestone Dec 21, 2022
@portante portante self-assigned this Dec 21, 2022
@portante
Copy link
Member Author

See issue #3124.

@portante portante merged commit 30ec6b3 into distributed-system-analysis:main Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants