chore: librarian update image pull request: 20251215T171601Z by daniel-sanche · Pull Request #1464 · googleapis/python-spanner · GitHub
Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

chore: librarian update image pull request: 20251215T171601Z#1464

Closed
daniel-sanche wants to merge 1 commit intomainfrom
librarian-20251215T171601Z
Closed

chore: librarian update image pull request: 20251215T171601Z#1464
daniel-sanche wants to merge 1 commit intomainfrom
librarian-20251215T171601Z

Conversation

@daniel-sanche
Copy link
Copy Markdown
Contributor

feat: update image to us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:eec191fc4904c204cd717c79812cd66997b5559776483ee223f69c8f43e99224

…prod/images-prod/python-librarian-generator@sha256:eec191fc4904c204cd717c79812cd66997b5559776483ee223f69c8f43e99224
@daniel-sanche daniel-sanche requested a review from a team as a code owner December 16, 2025 01:16
@daniel-sanche daniel-sanche requested review from a team and msampathkumar December 16, 2025 01:16
@product-auto-label product-auto-label Bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/python-spanner API. labels Dec 16, 2025
@gemini-code-assist
Copy link
Copy Markdown
Contributor

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 is an automated update from the librarian tool, primarily updating the generator image and regenerating a large number of files. The key changes include making OpenTelemetry dependencies optional under a tracing extra, removing support for Python 3.14 from test configurations, and refactoring mTLS client certificate handling. I've found a critical issue in the newly added version-check logic in __init__.py files that could lead to a NameError, and a minor formatting issue in setup.py. My review includes suggestions to fix these issues.

Comment on lines +105 to +192
else: # pragma: NO COVER
# An older version of api_core is installed which does not define the
# functions above. We do equivalent checks manually.
try:
import warnings
import sys

_py_version_str = sys.version.split()[0]
_package_label = "google.cloud.spanner_admin_database_v1"
if sys.version_info < (3, 9):
warnings.warn(
"You are using a non-supported Python version "
+ f"({_py_version_str}). Google will not post any further "
+ f"updates to {_package_label} supporting this Python version. "
+ "Please upgrade to the latest Python version, or at "
+ f"least to Python 3.9, and then update {_package_label}.",
FutureWarning,
)
if sys.version_info[:2] == (3, 9):
warnings.warn(
f"You are using a Python version ({_py_version_str}) "
+ f"which Google will stop supporting in {_package_label} in "
+ "January 2026. Please "
+ "upgrade to the latest Python version, or at "
+ "least to Python 3.10, before then, and "
+ f"then update {_package_label}.",
FutureWarning,
)

def parse_version_to_tuple(version_string: str):
"""Safely converts a semantic version string to a comparable tuple of integers.
Example: "4.25.8" -> (4, 25, 8)
Ignores non-numeric parts and handles common version formats.
Args:
version_string: Version string in the format "x.y.z" or "x.y.z<suffix>"
Returns:
Tuple of integers for the parsed version string.
"""
parts = []
for part in version_string.split("."):
try:
parts.append(int(part))
except ValueError:
# If it's a non-numeric part (e.g., '1.0.0b1' -> 'b1'), stop here.
# This is a simplification compared to 'packaging.parse_version', but sufficient
# for comparing strictly numeric semantic versions.
break
return tuple(parts)

def _get_version(dependency_name):
try:
version_string: str = metadata.version(dependency_name)
parsed_version = parse_version_to_tuple(version_string)
return (parsed_version, version_string)
except Exception:
# Catch exceptions from metadata.version() (e.g., PackageNotFoundError)
# or errors during parse_version_to_tuple
return (None, "--")

_dependency_package = "google.protobuf"
_next_supported_version = "4.25.8"
_next_supported_version_tuple = (4, 25, 8)
_recommendation = " (we recommend 6.x)"
(_version_used, _version_used_string) = _get_version(_dependency_package)
if _version_used and _version_used < _next_supported_version_tuple:
warnings.warn(
f"Package {_package_label} depends on "
+ f"{_dependency_package}, currently installed at version "
+ f"{_version_used_string}. Future updates to "
+ f"{_package_label} will require {_dependency_package} at "
+ f"version {_next_supported_version} or higher{_recommendation}."
+ " Please ensure "
+ "that either (a) your Python environment doesn't pin the "
+ f"version of {_dependency_package}, so that updates to "
+ f"{_package_label} can require the higher version, or "
+ "(b) you manually update your Python environment to use at "
+ f"least version {_next_supported_version} of "
+ f"{_dependency_package}.",
FutureWarning,
)
except Exception:
warnings.warn(
"Could not determine the version of Python "
+ "currently being used. To continue receiving "
+ "updates for {_package_label}, ensure you are "
+ "using a supported version of Python; see "
+ "https://devguide.python.org/versions/"
)
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.

critical

This else block has a couple of issues:

  1. NameError in except block: The _package_label variable is defined inside the try block (line 113). If an exception occurs before this line (e.g., on import sys), the except block will try to access _package_label and raise a NameError.
  2. Incorrect string formatting: The warning message in the except block (lines 187-191) uses string concatenation with + but includes "{_package_label}", which will be treated as a literal string instead of an interpolated variable.
  3. Style: The long warning messages are constructed with a mix of f-strings and + concatenation. Using a single, multi-line f-string would be more readable and consistent.

To resolve this, you should define _package_label before the try block and use f-strings for the warning messages.

else:  # pragma: NO COVER
    # An older version of api_core is installed which does not define the
    # functions above. We do equivalent checks manually.
    _package_label = "google.cloud.spanner_admin_database_v1"
    try:
        import warnings
        import sys

        _py_version_str = sys.version.split()[0]
        if sys.version_info < (3, 9):
            warnings.warn(
                (f"You are using a non-supported Python version ({_py_version_str}). "
                 f"Google will not post any further updates to {_package_label} "
                 f"supporting this Python version. Please upgrade to the latest "
                 f"Python version, or at least to Python 3.9, and then update "
                 f"{_package_label}."),
                FutureWarning,
            )
        if sys.version_info[:2] == (3, 9):
            warnings.warn(
                (f"You are using a Python version ({_py_version_str}) which Google "
                 f"will stop supporting in {_package_label} in January 2026. "
                 f"Please upgrade to the latest Python version, or at least to "
                 f"Python 3.10, before then, and then update {_package_label}."),
                FutureWarning,
            )

        def parse_version_to_tuple(version_string: str):
            """Safely converts a semantic version string to a comparable tuple of integers.
            Example: "4.25.8" -> (4, 25, 8)
            Ignores non-numeric parts and handles common version formats.
            Args:
                version_string: Version string in the format "x.y.z" or "x.y.z<suffix>"
            Returns:
                Tuple of integers for the parsed version string.
            """
            parts = []
            for part in version_string.split("."):
                try:
                    parts.append(int(part))
                except ValueError:
                    # If it's a non-numeric part (e.g., '1.0.0b1' -> 'b1'), stop here.
                    # This is a simplification compared to 'packaging.parse_version', but sufficient
                    # for comparing strictly numeric semantic versions.
                    break
            return tuple(parts)

        def _get_version(dependency_name):
            try:
                version_string: str = metadata.version(dependency_name)
                parsed_version = parse_version_to_tuple(version_string)
                return (parsed_version, version_string)
            except Exception:
                # Catch exceptions from metadata.version() (e.g., PackageNotFoundError)
                # or errors during parse_version_to_tuple
                return (None, "--")

        _dependency_package = "google.protobuf"
        _next_supported_version = "4.25.8"
        _next_supported_version_tuple = (4, 25, 8)
        _recommendation = " (we recommend 6.x)"
        (_version_used, _version_used_string) = _get_version(_dependency_package)
        if _version_used and _version_used < _next_supported_version_tuple:
            warnings.warn(
                (f"Package {_package_label} depends on {_dependency_package}, "
                 f"currently installed at version {_version_used_string}. Future "
                 f"updates to {_package_label} will require {_dependency_package} at "
                 f"version {_next_supported_version} or higher{_recommendation}. "
                 f"Please ensure that either (a) your Python environment doesn't "
                 f"pin the version of {_dependency_package}, so that updates to "
                 f"{_package_label} can require the higher version, or (b) you "
                 f"manually update your Python environment to use at least version "
                 f"{_next_supported_version} of {_dependency_package}."),
                FutureWarning,
            )
    except Exception:
        warnings.warn(
            f"Could not determine the version of Python currently being used. "
            f"To continue receiving updates for {_package_label}, ensure you are "
            f"using a supported version of Python; see "
            f"https://devguide.python.org/versions/"
        )

Comment on lines +85 to +172
else: # pragma: NO COVER
# An older version of api_core is installed which does not define the
# functions above. We do equivalent checks manually.
try:
import warnings
import sys

_py_version_str = sys.version.split()[0]
_package_label = "google.cloud.spanner_admin_instance_v1"
if sys.version_info < (3, 9):
warnings.warn(
"You are using a non-supported Python version "
+ f"({_py_version_str}). Google will not post any further "
+ f"updates to {_package_label} supporting this Python version. "
+ "Please upgrade to the latest Python version, or at "
+ f"least to Python 3.9, and then update {_package_label}.",
FutureWarning,
)
if sys.version_info[:2] == (3, 9):
warnings.warn(
f"You are using a Python version ({_py_version_str}) "
+ f"which Google will stop supporting in {_package_label} in "
+ "January 2026. Please "
+ "upgrade to the latest Python version, or at "
+ "least to Python 3.10, before then, and "
+ f"then update {_package_label}.",
FutureWarning,
)

def parse_version_to_tuple(version_string: str):
"""Safely converts a semantic version string to a comparable tuple of integers.
Example: "4.25.8" -> (4, 25, 8)
Ignores non-numeric parts and handles common version formats.
Args:
version_string: Version string in the format "x.y.z" or "x.y.z<suffix>"
Returns:
Tuple of integers for the parsed version string.
"""
parts = []
for part in version_string.split("."):
try:
parts.append(int(part))
except ValueError:
# If it's a non-numeric part (e.g., '1.0.0b1' -> 'b1'), stop here.
# This is a simplification compared to 'packaging.parse_version', but sufficient
# for comparing strictly numeric semantic versions.
break
return tuple(parts)

def _get_version(dependency_name):
try:
version_string: str = metadata.version(dependency_name)
parsed_version = parse_version_to_tuple(version_string)
return (parsed_version, version_string)
except Exception:
# Catch exceptions from metadata.version() (e.g., PackageNotFoundError)
# or errors during parse_version_to_tuple
return (None, "--")

_dependency_package = "google.protobuf"
_next_supported_version = "4.25.8"
_next_supported_version_tuple = (4, 25, 8)
_recommendation = " (we recommend 6.x)"
(_version_used, _version_used_string) = _get_version(_dependency_package)
if _version_used and _version_used < _next_supported_version_tuple:
warnings.warn(
f"Package {_package_label} depends on "
+ f"{_dependency_package}, currently installed at version "
+ f"{_version_used_string}. Future updates to "
+ f"{_package_label} will require {_dependency_package} at "
+ f"version {_next_supported_version} or higher{_recommendation}."
+ " Please ensure "
+ "that either (a) your Python environment doesn't pin the "
+ f"version of {_dependency_package}, so that updates to "
+ f"{_package_label} can require the higher version, or "
+ "(b) you manually update your Python environment to use at "
+ f"least version {_next_supported_version} of "
+ f"{_dependency_package}.",
FutureWarning,
)
except Exception:
warnings.warn(
"Could not determine the version of Python "
+ "currently being used. To continue receiving "
+ "updates for {_package_label}, ensure you are "
+ "using a supported version of Python; see "
+ "https://devguide.python.org/versions/"
)
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.

critical

This else block has a couple of issues:

  1. NameError in except block: The _package_label variable is defined inside the try block (line 93). If an exception occurs before this line (e.g., on import sys), the except block will try to access _package_label and raise a NameError.
  2. Incorrect string formatting: The warning message in the except block (lines 167-171) uses string concatenation with + but includes "{_package_label}", which will be treated as a literal string instead of an interpolated variable.
  3. Style: The long warning messages are constructed with a mix of f-strings and + concatenation. Using a single, multi-line f-string would be more readable and consistent.

To resolve this, you should define _package_label before the try block and use f-strings for the warning messages.

else:  # pragma: NO COVER
    # An older version of api_core is installed which does not define the
    # functions above. We do equivalent checks manually.
    _package_label = "google.cloud.spanner_admin_instance_v1"
    try:
        import warnings
        import sys

        _py_version_str = sys.version.split()[0]
        if sys.version_info < (3, 9):
            warnings.warn(
                (f"You are using a non-supported Python version ({_py_version_str}). "
                 f"Google will not post any further updates to {_package_label} "
                 f"supporting this Python version. Please upgrade to the latest "
                 f"Python version, or at least to Python 3.9, and then update "
                 f"{_package_label}."),
                FutureWarning,
            )
        if sys.version_info[:2] == (3, 9):
            warnings.warn(
                (f"You are using a Python version ({_py_version_str}) which Google "
                 f"will stop supporting in {_package_label} in January 2026. "
                 f"Please upgrade to the latest Python version, or at least to "
                 f"Python 3.10, before then, and then update {_package_label}."),
                FutureWarning,
            )

        def parse_version_to_tuple(version_string: str):
            """Safely converts a semantic version string to a comparable tuple of integers.
            Example: "4.25.8" -> (4, 25, 8)
            Ignores non-numeric parts and handles common version formats.
            Args:
                version_string: Version string in the format "x.y.z" or "x.y.z<suffix>"
            Returns:
                Tuple of integers for the parsed version string.
            """
            parts = []
            for part in version_string.split("."):
                try:
                    parts.append(int(part))
                except ValueError:
                    # If it's a non-numeric part (e.g., '1.0.0b1' -> 'b1'), stop here.
                    # This is a simplification compared to 'packaging.parse_version', but sufficient
                    # for comparing strictly numeric semantic versions.
                    break
            return tuple(parts)

        def _get_version(dependency_name):
            try:
                version_string: str = metadata.version(dependency_name)
                parsed_version = parse_version_to_tuple(version_string)
                return (parsed_version, version_string)
            except Exception:
                # Catch exceptions from metadata.version() (e.g., PackageNotFoundError)
                # or errors during parse_version_to_tuple
                return (None, "--")

        _dependency_package = "google.protobuf"
        _next_supported_version = "4.25.8"
        _next_supported_version_tuple = (4, 25, 8)
        _recommendation = " (we recommend 6.x)"
        (_version_used, _version_used_string) = _get_version(_dependency_package)
        if _version_used and _version_used < _next_supported_version_tuple:
            warnings.warn(
                (f"Package {_package_label} depends on {_dependency_package}, "
                 f"currently installed at version {_version_used_string}. Future "
                 f"updates to {_package_label} will require {_dependency_package} at "
                 f"version {_next_supported_version} or higher{_recommendation}. "
                 f"Please ensure that either (a) your Python environment doesn't "
                 f"pin the version of {_dependency_package}, so that updates to "
                 f"{_package_label} can require the higher version, or (b) you "
                 f"manually update your Python environment to use at least version "
                 f"{_next_supported_version} of {_dependency_package}."),
                FutureWarning,
            )
    except Exception:
        warnings.warn(
            f"Could not determine the version of Python currently being used. "
            f"To continue receiving updates for {_package_label}, ensure you are "
            f"using a supported version of Python; see "
            f"https://devguide.python.org/versions/"
        )

Comment thread setup.py
"opentelemetry-semantic-conventions >= 0.43b0",
"opentelemetry-resourcedetector-gcp >= 1.8.0a0",
"google-cloud-monitoring >= 2.16.0",
"mmh3 >= 4.1.0 ",
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

There is a trailing space in this dependency string. While it might not cause issues with all tools, it's best to remove it for correctness and to avoid potential parsing problems. Also, adding a trailing comma is good practice for list items to make future diffs cleaner.

Suggested change

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: spanner Issues related to the googleapis/python-spanner API. size: l Pull request size is large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants