feat(gapic): support mTLS certificates when available #1467
Conversation
…prod/images-prod/python-librarian-generator@sha256:b8058df4c45e9a6e07f6b4d65b458d0d059241dd34c814f151c8bf6b89211209
product-auto-label
bot
added
the
api: spanner
Summary of ChangesHello @daniel-sanche, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces Python 3.14 support, integrates OpenTelemetry as a core dependency, and updates the librarian generator image. A significant portion of the changes involves refactoring and enhancing the mTLS client certificate handling logic across Spanner client libraries, along with corresponding test updates. Additionally, several outdated keyword fixup scripts and testing constraints have been removed to streamline the codebase. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request appears to be an automated update to add support for Python 3.14 and refresh dependencies. The changes include updating Python versions in noxfile.py, moving OpenTelemetry dependencies to core, and refactoring mTLS client certificate handling.
My main concerns are:
- The OpenTelemetry dependencies have been moved from an optional
tracingextra to core dependencies. This significantly increases the dependency footprint for all users and should be reverted if unintentional. - There is a large block of duplicated code for version checking in
spanner_admin_database_v1/__init__.pyandspanner_admin_instance_v1/__init__.pywhich also uses overly broad exception handling. This should be refactored for better maintainability. - A
-sflag was added topytestcalls, which could make CI logs overly verbose.
I've added specific comments with suggestions for these points.
| session.run( | ||
| "py.test", | ||
| "--quiet", | ||
| "-s", |
There was a problem hiding this comment.
The -s flag (short for --capture=no) disables output capturing in pytest. While this can be useful for local debugging, it often makes CI logs excessively verbose. It's generally recommended to avoid this flag in CI configurations to keep logs concise and focused on failures. Please consider removing this flag.
| 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/" | ||
| ) | ||
|
|
There was a problem hiding this comment.
This large block of version-checking code is duplicated in google/cloud/spanner_admin_instance_v1/__init__.py. To improve maintainability, this logic should be extracted into a shared utility module.
Additionally, the except Exception: clauses on lines 155 and 181 are too broad. They can hide unexpected errors. Please consider catching more specific exceptions. For example:
- On line 155, you could catch
(AttributeError, ValueError, ImportError). - On line 181, you could catch
ImportError.
| 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/" | ||
| ) | ||
|
|
There was a problem hiding this comment.
This large block of version-checking code is duplicated from google/cloud/spanner_admin_database_v1/__init__.py. To improve maintainability, this logic should be extracted into a shared utility module.
Additionally, the except Exception: clauses are too broad and can hide unexpected errors. Please consider catching more specific exceptions.
daniel-sanche
changed the title
PR created by the Librarian CLI to initialize a release. Merging this PR will auto trigger a release. Librarian Version: v0.7.0 Language Image: us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:b8058df4c45e9a6e07f6b4d65b458d0d059241dd34c814f151c8bf6b89211209 <details><summary>google-cloud-spanner: 3.61.0</summary> ## [3.61.0](v3.60.0...v3.61.0) (2025-12-16) ### Features * support mTLS certificates when available (#1467) ([df87c3e](df87c3ed)) </details>
3 participants
feat: update image to us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:b8058df4c45e9a6e07f6b4d65b458d0d059241dd34c814f151c8bf6b89211209