Skip to content

feat(sqlalchemy-spanner): wire timeout execution option through to DBAPI Connection.timeout - #16468

Open
waiho-gumloop wants to merge 1 commit into
googleapis:mainfrom
waiho-gumloop:feat/sqlalchemy-spanner-timeout-option
Open

feat(sqlalchemy-spanner): wire timeout execution option through to DBAPI Connection.timeout#16468
waiho-gumloop wants to merge 1 commit into
googleapis:mainfrom
waiho-gumloop:feat/sqlalchemy-spanner-timeout-option

Conversation

@waiho-gumloop

Copy link
Copy Markdown
Contributor

Summary

Add timeout handling to SpannerExecutionContext.pre_exec() and reset_connection() so that users can set a per-statement gRPC deadline via SQLAlchemy's execution_options(timeout=N).

Fixes#16467

Background

The Spanner SQLAlchemy dialect reads execution options in SpannerExecutionContext.pre_exec() and forwards them to the DBAPI connection. Currently handled options: read_only, staleness, request_priority, transaction_tag, request_tag, ignore_transaction_warnings. The timeout option is not handled.

The underlying Spanner client's _SnapshotBase.execute_sql() accepts a timeout parameter that controls the gRPC deadline for ExecuteStreamingSql. Without it, all queries use the gRPC default of 3600 seconds.

Timeline

DateCommitEvent
Apr 2020Initial commitDialect created — pre_exec() handles read_only and staleness
Oct 2021d976fda (PR googleapis/python-spanner#269)request_priority added to pre_exec()
Jan 20235bd5076 (PR googleapis/python-spanner#494)transaction_tag and request_tag added to pre_exec()
Jun 202366c32a4 (PR googleapis/python-spanner#503)ignore_transaction_warnings added to pre_exec()
NoneN/Atimeout was never added to pre_exec() or reset_connection()

Each new execution option was added incrementally. timeout was not included in any of these additions.

Changes

sqlalchemy_spanner.py

  • Add timeout handling in pre_exec(): reads self.execution_options.get("timeout") and sets self._dbapi_connection.connection.timeout
  • Add timeout reset in reset_connection(): sets dbapi_conn.timeout = None when connection returns to pool

Usage

fromsqlalchemyimportcreate_engineengine=create_engine("spanner:///...")
# Per-connection timeoutwithengine.connect().execution_options(timeout=60) asconn:
conn.execute(text("SELECT * FROM my_table"))
# Engine-level default timeoutengine=create_engine("spanner:///...", execution_options={"timeout": 60})

Tests

Added 5 unit tests in tests/unit/test_sqlalchemy_spanner.py:

  • test_reset_connection_clears_timeout — verifies reset_connection sets timeout = None
  • test_reset_connection_with_wrapper — verifies reset works through connection wrapper
  • test_pre_exec_sets_timeout — verifies timeout is forwarded to DBAPI connection
  • test_pre_exec_no_timeout_leaves_connection_unchanged — verifies timeout is not set when absent from options
  • test_pre_exec_timeout_with_other_options — verifies timeout coexists with read_only and request_priority

Prerequisites

This change depends on the DBAPI Connection supporting a timeout property: #16492 (PR: googleapis/python-spanner#1535)

Related

@waiho-gumloop
waiho-gumloop requested review from a team as code ownersMarch 27, 2026 01:52

@gemini-code-assistgemini-code-assistBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a timeout execution option for Spanner connections and ensures the timeout is cleared during connection resets. It also includes unit tests for these changes. Review feedback recommends using unittest assertion methods for improved failure messages and moving local imports to the top of the file to comply with PEP 8.

Comment on lines +31 to +33
assert dbapi_conn.staleness is None
assert dbapi_conn.read_only is False
assert dbapi_conn.timeout is None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In classes that inherit from unittest.TestCase, it is a best practice to use the provided assertion methods (e.g., self.assertIsNone, self.assertFalse, self.assertEqual). These methods provide more descriptive failure messages, which can make debugging failing tests easier. Please consider replacing the assert statements throughout this file with their unittest counterparts.

Suggested change
assertdbapi_conn.stalenessisNone
assertdbapi_conn.read_onlyisFalse
assertdbapi_conn.timeoutisNone
self.assertIsNone(dbapi_conn.staleness)
self.assertFalse(dbapi_conn.read_only)
self.assertIsNone(dbapi_conn.timeout)

Comment on lines +50 to +52
from google.cloud.sqlalchemy_spanner.sqlalchemy_spanner import (
SpannerExecutionContext,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To align with PEP 8 guidelines and improve code organization, please move this import to the top of the file. Local imports are generally reserved for cases like avoiding circular dependencies or handling optional packages, which doesn't seem to be the case here. You can add SpannerExecutionContext to the existing import from google.cloud.sqlalchemy_spanner.sqlalchemy_spanner on line 21.

References
  1. PEP 8 states that imports should always be put at the top of the file, just after any module comments and docstrings, and before module globals and constants. This makes it clear what modules the file requires. (link)

…API Connection.timeout
Add timeout handling to SpannerExecutionContext.pre_exec() and
reset_connection() so that users can set a per-statement gRPC deadline
via execution_options(timeout=N).
Depends on googleapis/python-spanner#1534 for the DBAPI Connection.timeout property.
Fixesgoogleapis#16467
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(sqlalchemy-spanner): wire timeout execution option through to DBAPI Connection.timeout

1 participant

@waiho-gumloop