FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat(dbapi): wire timeout parameter through Connection to execute_sql by waiho-gumloop · Pull Request #16497 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

feat(dbapi): wire timeout parameter through Connection to execute_sql - #16497

Open
waiho-gumloop wants to merge 1 commit into
googleapis:mainfrom
waiho-gumloop:feat/dbapi-timeout-parameter
ConversationCommits (1)ChecksFiles changed
Open

feat(dbapi): wire timeout parameter through Connection to execute_sql#16497
waiho-gumloop wants to merge 1 commit into
googleapis:mainfrom
waiho-gumloop:feat/dbapi-timeout-parameter

Conversation

waiho-gumloop commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Add a timeout property to Connection and pass timeout= to execute_sql() in the three DBAPI code paths that currently omit it: snapshot reads, transaction statements, and autocommit DML.

Fixes #16492

Background

_SnapshotBase.execute_sql() accepts a timeout parameter that controls the gRPC deadline for the ExecuteStreamingSql RPC. When not provided, it defaults to gapic_v1.method.DEFAULT, which resolves to default_timeout=3600.0 in the transport layer.

The DBAPI calls execute_sql() in three locations, none of which pass timeout=:

  1. Snapshot reads -- cursor._handle_DQL_with_snapshot() calls snapshot.execute_sql(sql, params, param_types, request_options=...) without timeout=
  2. Transaction reads/writes -- connection.run_statement() calls transaction.execute_sql(sql, params, param_types=..., request_options=...) without timeout=
  3. Autocommit DML -- cursor._do_execute_update_in_autocommit() calls transaction.execute_sql(sql, params=..., param_types=..., last_statement=True) without timeout=

This means DBAPI consumers (SQLAlchemy, Django, raw DBAPI) cannot control the gRPC deadline for individual statements.

Changes

connection.py

  • Add self._timeout = None to __init__
  • Add timeout property and setter
  • Pass timeout=self._timeout in run_statement() when set

cursor.py

  • Pass timeout=self.connection._timeout in _handle_DQL_with_snapshot() when set
  • Pass timeout=self.connection._timeout in _do_execute_update_in_autocommit() when set

When timeout is None (the default), timeout= is not passed, preserving the existing behavior of using gapic_v1.method.DEFAULT.

Usage

from google.cloud.spanner_dbapi import connect

conn = connect(instance_id, database_id, project=project)
conn.timeout = 60  # 60-second gRPC deadline for subsequent statements

cursor = conn.cursor()
cursor.execute("SELECT * FROM my_table")

Tests

Added 7 unit tests covering default value, property getter/setter, and timeout propagation in all three code paths.

Related

feat(dbapi): wire timeout parameter through Connection to execute_sql
861d7e9 
Add a timeout property to Connection and pass timeout= to execute_sql()
in the three DBAPI code paths that currently omit it: snapshot reads,
transaction statements, and autocommit DML.

When timeout is None (the default), timeout= is not passed, preserving
the existing behavior of using gapic_v1.method.DEFAULT (3600s).

Fixes googleapis#16492
waiho-gumloop requested review from a team as code owners April 1, 2026 00:19
gemini-code-assist Bot reviewed Apr 1, 2026
View reviewed changes

gemini-code-assist Bot 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Code Review

This pull request introduces a timeout property to the Spanner DB-API Connection class, allowing users to specify gRPC deadlines for SQL operations. The timeout is propagated to execute_sql calls within both Connection and Cursor methods, and comprehensive unit tests have been added to verify this functionality. Feedback was provided to use the public timeout property instead of the private _timeout attribute for better encapsulation and consistency across the codebase.

Comment thread packages/google-cloud-spanner/google/cloud/spanner_dbapi/connection.py
Comment on lines +592 to +593
if self._timeout is not None:
kwargs["timeout"] = self._timeout

gemini-code-assist Bot Apr 1, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Consider using the public timeout property instead of the private _timeout attribute for consistency, even within the class methods.

Suggested change
if self._timeout is not None:
kwargs["timeout"] = self._timeout
if self.timeout is not None:
kwargs["timeout"] = self.timeout

Comment thread packages/google-cloud-spanner/google/cloud/spanner_dbapi/cursor.py
Comment on lines +236 to +237
if self.connection._timeout is not None:
kwargs["timeout"] = self.connection._timeout

gemini-code-assist Bot Apr 1, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It is better to use the public timeout property of the connection instead of accessing the private _timeout attribute directly. This ensures better encapsulation and consistency with how other connection properties (like autocommit or read_only) are accessed throughout the codebase.

Suggested change
if self.connection._timeout is not None:
kwargs["timeout"] = self.connection._timeout
if self.connection.timeout is not None:
kwargs["timeout"] = self.connection.timeout

Comment thread packages/google-cloud-spanner/google/cloud/spanner_dbapi/cursor.py
Comment on lines +554 to +555
if self.connection._timeout is not None:
kwargs["timeout"] = self.connection._timeout

gemini-code-assist Bot Apr 1, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It is better to use the public timeout property of the connection instead of accessing the private _timeout attribute directly. This ensures better encapsulation and consistency with how other connection properties are accessed.

Suggested change
if self.connection._timeout is not None:
kwargs["timeout"] = self.connection._timeout
if self.connection.timeout is not None:
kwargs["timeout"] = self.connection.timeout

waiho-gumloop mentioned this pull request Apr 1, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat(dbapi): wire timeout parameter through to execute_sql in cursor and connection

1 participant

Back | FazBrowse Home | New Git URL