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

[waiho-gumloop]

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

Date	Commit	Event
Apr 2020	Initial commit	Dialect created — pre_exec() handles read_only and staleness
Oct 2021	d976fda (PR #269)	request_priority added to pre_exec()
Jan 2023	5bd5076 (PR #494)	transaction_tag and request_tag added to pre_exec()
Jun 2023	66c32a4 (PR #503)	ignore_transaction_warnings added to pre_exec()
None	N/A	timeout 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

from sqlalchemy import create_engine

engine = create_engine("spanner:///...")

# Per-connection timeout
with engine.connect().execution_options(timeout=60) as conn:
    conn.execute(text("SELECT * FROM my_table"))

# Engine-level default timeout
engine = 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: googleapis/python-spanner#1534 (PR: googleapis/python-spanner#1535)

Related

• DBAPI issue: feat(dbapi): wire timeout parameter through to execute_sql in cursor and connection python-spanner#1534
• DBAPI PR: feat(dbapi): wire timeout parameter through Connection to execute_sql python-spanner#1535
• _SnapshotBase.execute_sql() has accepted timeout= since November 2018 (PR [Spanner] Add timeout + retry settings to Sessions/Snapshots #6536 in the original monorepo)
• This code was previously in googleapis/python-spanner-sqlalchemy (now archived)

[gemini-code-assist]

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.

[gemini-code-assist]

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

	assert dbapi_conn.staleness is None
	assert dbapi_conn.read_only is False
	assert dbapi_conn.timeout is None
	self.assertIsNone(dbapi_conn.staleness)
	self.assertFalse(dbapi_conn.read_only)
	self.assertIsNone(dbapi_conn.timeout)

[gemini-code-assist]

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)