feat: Add support for python 312 (ads templates) #1861
Conversation
parthea
force-pushed
the
add-support-for-python-312-ads
branch
3 times, most recently
from
e2540b2 to
a00ecc7
Compare
parthea
force-pushed
the
add-support-for-python-312-ads
branch
from
a00ecc7 to
9de4153
Compare
parthea
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
| ) | ||
| except pkg_resources.DistributionNotFound: | ||
| DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo() | ||
| DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo(gapic_version=package_version.__version__) |
There was a problem hiding this comment.
It looks like this is grabbing the version that's defined in the above gapic_version file, is that correct?
The implementation is actually to access the version of the Ads client library itself, see the logic here
If you look at go/ads-client-libs-user-agent-design you can see that we opted to use the gccl user agent k/v for our own logging purposes, which is why the gapic_version keyword arg is used here instead of something else.
Is it possible to retrieve the client library version without pkg_resources? If not, then we might need to add that logic in our interceptors before this change can be made.
There was a problem hiding this comment.
It looks like this is grabbing the version that's defined in the above gapic_version file, is that correct?
Yes, that is correct. In google-cloud-python, we use release-please to update the version in gapic_version.py prior to releasing. As an example see googleapis/google-cloud-python#12010.
The implementation is actually to access the version of the Ads client library itself, see the logic here
We've moved away from this in GAPIC, and chose to populate gapic_version.py instead of a hardcoded version number in setup.py. Customers can obtain the version number programatically using __version__.
Is it possible to retrieve the client library version without pkg_resources? If not, then we might need to add that logic in our interceptors before this change can be made.
It is possible to use something else like https://docs.python.org/3/library/importlib.metadata.html but we chose to have a version module instead.
| packages=packages, | ||
| python_requires=">=3.7", | ||
| install_requires=dependencies, | ||
| include_package_data=True, |
There was a problem hiding this comment.
We don't use the generated setup.py file, so I'm not sure it makes sense to make these changes, and perhaps you could remove it from the templates.
There was a problem hiding this comment.
This file is still needed so that we can run tests for the ads templates. We'll need to maintain it as long as we support ads templates.
Fixes #1858 🦕
Fixes #1859 🦕
This PR:
execin both ads and standard templatessetup.pyof Ads to be similar to the standard templatespkg_resourcesgapic_version.pyto Ads templates__version__to__init__.pyfor Ads templates