Skip to content

Spanner Dialect Handles Pool Reset Events for Other Databases #706

Closed
@waltaskew

Description

@waltaskew

This event listener in the dialect

@listens_for(Pool, "reset")
def reset_connection(dbapi_conn, connection_record, reset_state=None):
    ...

will fire for all pool reset events, even if the connection comes from an engine for another dialect. This leads to errors if other dialects aren't expecting their connections to run through this handler.

This happens when working with multiple databases at once, for instance

bq_engine = create_engine("bigquery://project-id")
spanner_engine = create_engine(
    "spanner+spanner:///projects/project-id/instances/instance-id/databases/database-id"
)
with bq_engine.connect() as conn:
    ...

this will throw errors as the BigQuery connection is pushed through the spanner dialect's event handler

AttributeError: 'Connection' object has no attribute 'rollback'

I'm not exactly sure how dialect authors are expected to handle this. I think it'd either require some checking in the event handler to make sure the connection came from an engine for the dialect or registering the event handler only for the dialect's engines & pools rather than globally. The CreateEnginePlugin might be useful for this?

class SpannerPlugin(CreateEnginePlugin):
    def engine_created(self, engine):
        event.listen(engine, "reset", reset_connection)

I couldn't find examples of how other dialects register event handlers, so I'm not sure what the best path is here. Maybe dialect authors are expected to implement this some other way without event handlers?

It's also possible the event handler could just not contain the final else: dbapi_conn.rollback() call
and leave the rollback call to the pool's reset_on_return behaviour, which I'd guess would be calling rollback already? Then the event handler would only be attempting to work with spanner_dbapi.Connection objects.

Metadata

Metadata

Assignees

Labels

api: spannerIssues related to the googleapis/python-spanner-sqlalchemy API.priority: p2Moderately-important priority. Fix may not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions