Skip to content

test(spanner): disable pooling in mock server tests#16655

Closed
chalmerlowe wants to merge 1 commit into
chore-update-to-rufffrom
fix-spanner-mock-tests
Closed

test(spanner): disable pooling in mock server tests#16655
chalmerlowe wants to merge 1 commit into
chore-update-to-rufffrom
fix-spanner-mock-tests

Conversation

@chalmerlowe
Copy link
Copy Markdown
Contributor

This PR disables connection pooling in the mock server tests for sqlalchemy-spanner.

We observed that many tests were failing with an off-by-one error in the expected number of requests (e.g., expecting 6 but getting 5). This suggests that CreateSessionRequest was not being sent for every test, likely because sessions were being reused from the pool across tests.

By setting poolclass=NullPool in create_engine, we force a new connection and session for every test, ensuring predictable request counts.

@chalmerlowe chalmerlowe requested a review from a team as a code owner April 15, 2026 13:41
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

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 reorganizes imports and disables connection pooling in the create_engine method by using NullPool to ensure fresh sessions for each test. The review feedback recommends further organizing the import block to comply with PEP 8 alphabetical sorting and moving the NullPool import from the local method scope to the top level to improve code maintainability.

Comment on lines 16 to +30
import google.cloud.spanner_v1.types.result_set as result_set
import google.cloud.spanner_v1.types.type as spanner_type
import grpc
from google.api_core.client_options import ClientOptions
from google.auth.credentials import AnonymousCredentials
from google.cloud.spanner_dbapi.parsed_statement import AutocommitDmlMode
from google.cloud.spanner_v1 import (
Client,
ResultSet,
TypeCode,
)
from google.cloud.spanner_v1.database import Database
from google.cloud.spanner_v1.instance import Instance
import grpc
from sqlalchemy import Engine, create_engine
from sqlalchemy.testing.plugin.plugin_base import fixtures
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

The import block is disorganized and does not follow PEP 8 alphabetical sorting. Additionally, NullPool should be imported at the top level rather than locally within a method to improve readability and maintainability.

Suggested change
import google.cloud.spanner_v1.types.result_set as result_set
import google.cloud.spanner_v1.types.type as spanner_type
import grpc
from google.api_core.client_options import ClientOptions
from google.auth.credentials import AnonymousCredentials
from google.cloud.spanner_dbapi.parsed_statement import AutocommitDmlMode
from google.cloud.spanner_v1 import (
Client,
ResultSet,
TypeCode,
)
from google.cloud.spanner_v1.database import Database
from google.cloud.spanner_v1.instance import Instance
import grpc
from sqlalchemy import Engine, create_engine
from sqlalchemy.testing.plugin.plugin_base import fixtures
from google.api_core.client_options import ClientOptions
from google.auth.credentials import AnonymousCredentials
from google.cloud.spanner_dbapi.parsed_statement import AutocommitDmlMode
from google.cloud.spanner_v1 import (
Client,
ResultSet,
TypeCode,
)
from google.cloud.spanner_v1.database import Database
from google.cloud.spanner_v1.instance import Instance
import google.cloud.spanner_v1.types.result_set as result_set
import google.cloud.spanner_v1.types.type as spanner_type
import grpc
from sqlalchemy import Engine, NullPool, create_engine
from sqlalchemy.testing.plugin.plugin_base import fixtures

Comment on lines +165 to 166
from sqlalchemy.pool import NullPool
return create_engine(
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

Remove the local import of NullPool as it has been moved to the top-level imports.

        return create_engine(

@chalmerlowe chalmerlowe added this to the Update nox sessions milestone May 22, 2026
Sign up for free to 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.

1 participant