-
Notifications
You must be signed in to change notification settings - Fork 92
feat: provide and use Python version support check #832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
05ee669
8d1ade4
6d1058d
0f8caa2
3ec85a6
240baec
ee4d6b6
0726cd2
8f54a81
9f3d7ef
b03aa68
3bae70e
73de862
1ec82cb
79090e4
c123f1c
1c9aae5
b41cd7f
6faca07
0d03e1f
5a2cce0
cc41275
df92992
81eae62
d558af4
566b933
6adc2f3
e957094
0cf15f2
d60d12d
f3edf46
d22c43c
0fdd8bc
38ded4e
7dc906d
2de943a
e61455a
ce216e4
695bfcf
eebbe61
670ba62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,22 @@ | |
This package contains common code and utilities used by Google client libraries. | ||
""" | ||
|
||
from google.api_core import _python_package_support | ||
from google.api_core import _python_version_support | ||
from google.api_core import version as api_core_version | ||
|
||
__version__ = api_core_version.__version__ | ||
|
||
# NOTE: Until dependent artifacts require this version of | ||
# google.api_core, the functionality below must be made available | ||
# manually in those artifacts. | ||
|
||
check_python_version = _python_version_support.check_python_version | ||
check_python_version(package="google.api_core") | ||
|
||
check_dependency_versions = _python_package_support.check_dependency_versions | ||
check_dependency_versions("google.api_core") | ||
|
||
warn_deprecation_for_versions_less_than = ( | ||
_python_package_support.warn_deprecation_for_versions_less_than | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are both |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,175 @@ | ||
# Copyright 2025 Google LLC | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
"""Code to check versions of dependencies used by Google Cloud Client Libraries.""" | ||
|
||
import warnings | ||
import sys | ||
from typing import Optional, Tuple | ||
from ._python_version_support import ( | ||
_flatten_message, | ||
_get_distribution_and_import_packages, | ||
) | ||
|
||
from packaging.version import parse as parse_version, Version as PackagingVersion | ||
|
||
|
||
def get_dependency_version( | ||
dependency_name: str, | ||
) -> Tuple[Optional[PackagingVersion], str]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would a namedtuple be overkill here? |
||
"""Get the parsed version of an installed package dependency. | ||
This function checks for an installed package and returns its version | ||
as a `packaging.version.Version` object for safe comparison. It handles | ||
both modern (Python 3.8+) and legacy (Python 3.7) environments. | ||
Args: | ||
dependency_name: The distribution name of the package (e.g., 'requests'). | ||
Returns: | ||
A tuple containing the `packaging.version.Version` object and the | ||
version string, or `(None, '--')` if the package is not found or | ||
another error occurs during version discovery. | ||
""" | ||
try: | ||
if sys.version_info >= (3, 8): | ||
from importlib import metadata | ||
|
||
version_string = metadata.version(dependency_name) | ||
return (parse_version(version_string), version_string) | ||
|
||
# TODO(https://github.com/googleapis/python-api-core/issues/835): Remove | ||
# this code path once we drop support for Python 3.7 | ||
else: # pragma: NO COVER | ||
# Use pkg_resources, which is part of setuptools. | ||
import pkg_resources | ||
|
||
version_string = pkg_resources.get_distribution(dependency_name).version | ||
return (parse_version(version_string), version_string) | ||
|
||
except Exception: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we know what exceptions to expect? Or does this need to be broad? |
||
return (None, "--") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This magic "--" seems like it could cause trouble. Could we make the whole output optional if there's an exception, and let callers decide how to deal with an empty value? (or even just raise an exception here for the caller to handle?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we do keep the "--", can we make it a constant? |
||
|
||
|
||
def warn_deprecation_for_versions_less_than( | ||
dependent_import_package: str, | ||
dependency_import_package: str, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: it might be nice if these variables were more distinct. Maybe |
||
next_supported_version: str, | ||
recommended_version: Optional[str] = None, | ||
message_template: Optional[str] = None, | ||
): | ||
"""Issue any needed deprecation warnings for `dependency_import_package`. | ||
If `dependency_import_package` is installed at a version less than | ||
`next_supported_version`, this issues a warning using either a | ||
default `message_template` or one provided by the user. The | ||
default `message_template` informs the user that they will not receive | ||
future updates for `dependent_import_package` if | ||
`dependency_import_package` is somehow pinned to a version lower | ||
than `next_supported_version`. | ||
Args: | ||
dependent_import_package: The import name of the package that | ||
needs `dependency_import_package`. | ||
dependency_import_package: The import name of the dependency to check. | ||
next_supported_version: The dependency_import_package version number | ||
below which a deprecation warning will be logged. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could this be minimum_supported_version? Or am I misunderstanding it? |
||
recommended_version: If provided, the recommended next version, which | ||
could be higher than `next_supported_version`. | ||
message_template: A custom default message template to replace | ||
the default. This `message_template` is treated as an | ||
f-string, where the following variables are defined: | ||
`dependency_import_package`, `dependent_import_package` and | ||
`dependency_distribution_package` and | ||
`dependent_distribution_package` and `dependency_package`, | ||
`dependent_package` , which contain the import packages, the | ||
distribution packages, and pretty string with both the | ||
distribution and import packages for the dependency and the | ||
dependent, respectively; and `next_supported_version`, | ||
`version_used`, and `version_used_string`, which refer to supported | ||
and currently-used versions of the dependency. | ||
""" | ||
if ( | ||
not dependent_import_package | ||
or not dependency_import_package | ||
or not next_supported_version | ||
): # pragma: NO COVER | ||
return | ||
(version_used, version_used_string) = get_dependency_version( | ||
dependency_import_package | ||
) | ||
if not version_used: | ||
return | ||
if version_used < parse_version(next_supported_version): | ||
( | ||
dependency_package, | ||
dependency_distribution_package, | ||
) = _get_distribution_and_import_packages(dependency_import_package) | ||
( | ||
dependent_package, | ||
dependent_distribution_package, | ||
) = _get_distribution_and_import_packages(dependent_import_package) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can these variables be made less similar? (dependent_x vs dependency_x) |
||
|
||
recommendation = ( | ||
" (we recommend {recommended_version})" if recommended_version else "" | ||
) | ||
message_template = message_template or _flatten_message( | ||
""" | ||
DEPRECATION: Package {dependent_package} depends on | ||
{dependency_package}, currently installed at version | ||
{version_used_string}. Future updates to | ||
{dependent_package} will require {dependency_package} at | ||
version {next_supported_version} or | ||
higher{recommendation}. Please ensure that either (a) your | ||
Python environment doesn't pin the version of | ||
{dependency_package}, so that updates to | ||
{dependent_package} can require the higher version, or (b) | ||
you manually update your Python environment to use at | ||
least version {next_supported_version} of | ||
{dependency_package}. | ||
""" | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we make this into a constant? Maybe DEFAULT_PACKAGE_DEPRECATION_TEMPLATE? |
||
warnings.warn( | ||
message_template.format( | ||
dependent_import_package=dependent_import_package, | ||
dependency_import_package=dependency_import_package, | ||
dependent_distribution_package=dependent_distribution_package, | ||
dependency_distribution_package=dependency_distribution_package, | ||
dependency_package=dependency_package, | ||
dependent_package=dependent_package, | ||
next_supported_version=next_supported_version, | ||
recommendation=recommendation, | ||
version_used=version_used, | ||
version_used_string=version_used_string, | ||
), | ||
FutureWarning, | ||
) | ||
|
||
|
||
def check_dependency_versions(dependent_import_package: str): | ||
"""Bundle checks for all package dependencies. | ||
This function can be called by all dependents of google.api_core, | ||
to emit needed deprecation warnings for any of their | ||
dependencies. The dependencies to check should be updated here. | ||
Args: | ||
dependent_import_package: The distribution name of the calling package, whose | ||
dependencies we're checking. | ||
""" | ||
warn_deprecation_for_versions_less_than( | ||
dependent_import_package, "google.protobuf", "4.25.8", recommended_version="6.x" | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the idea that we will go back and manually add dependencies here in the future? If so, this feels a little hidden. Can we read this from a config file, or declare it as a constant at the top of the file or something? something like:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It kind of feels like dependencies should be given as an argument too? Can was assume all sdks will want to check against the same ones? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can this be re-arranged with comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And maybe the check methods themselves should be renamed to make it clear that they emit a warning. If I saw it imported in an external package, that part might not be clear from the name. But maybe I'm overthinking it