fix: don't rollback non-spanner connections on reset#709
fix: don't rollback non-spanner connections on reset#709olavloite merged 1 commit intogoogleapis:mainfrom
Conversation
This is the simplest way to address bug googleapis#706 and address the `AttributeError: 'Connection' object has no attribute 'rollback'` I'm seeing when connections from BigQuery engines get caught by this event handler. I think we should be able to rely on SQLAlchemy's [reset on return](https://docs.sqlalchemy.org/en/20/core/pooling.html#reset-on-return) behaviour to rollback transactions when they're reset rather than call rollback ourselves. That behaviour is also something end users can configure, so it'd be good to respect their settings if they disable the behaviour. Fixes: googleapis#706
product-auto-label
bot
added
the
api: spanner
olavloite
changed the title
|
@waltaskew small nit regarding commit messages / pull request titles: These are used to automatically trigger releases and generate the release notes. For that, we use Conventional Commits. The prefix that should be used for bug fixes is |
olavloite
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
olavloite
left a comment
olavloite
left a comment
There was a problem hiding this comment.
I agree that Spanner should not do anything with non-Spanner connections. I've manually verified that the default behavior of SQLAlchemy is to rollback on reset. That default is being executed after this event handler has been executed, meaning that non-Spanner connections get two rollback requests.
Got it -- sorry about that! |
4 participants
This is the simplest way to address bug #706 and address the
AttributeError: 'Connection' object has no attribute 'rollback'I'm seeing when connections from BigQuery engines get caught by this event handler.I think we should be able to rely on SQLAlchemy's reset on return behaviour to rollback transactions when they're reset rather than call rollback ourselves. That behaviour is also something end users can configure, so it'd be good to respect their settings if they disable the behaviour.
Fixes: #706