Skip to content

prototype: generator update preview#16982

Draft
jskeet wants to merge 2 commits intogoogleapis:mainfrom
jskeet:test-generator-update
Draft

prototype: generator update preview#16982
jskeet wants to merge 2 commits intogoogleapis:mainfrom
jskeet:test-generator-update

Conversation

@jskeet
Copy link
Copy Markdown
Contributor

@jskeet jskeet commented May 7, 2026

This PR is intended to demonstrate the results from updating librarian to use gapic-generator 1.31.0, so they can be reviewed before we're actually ready to make the change.

The first commit consists of handwritten changes to the post-processor string replacement scripts. The second commit is the result of running librarian generate --all when using gapic-generator 1.31.0.

@jskeet jskeet added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 7, 2026
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 updates dependency versions (e.g., google-api-core, grpcio) across librarian configuration files, removes multiple testing/constraints-3.9.txt references, and updates copyright years to 2026 for the google-ads-admanager package. Additionally, Python version support checks in admanager_v1/init.py are updated to target Python 3.10. Review feedback identifies a critical NameError in admanager_v1/init.py caused by the removal of a necessary sys import and raises concerns about unpinning dependencies in storage-integration.yaml, which may lead to non-deterministic CI behavior.

Comment on lines 501 to 503
import warnings

_py_version_str = sys.version.split()[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.

high

The import sys statement was removed from this block, but sys is still accessed on lines 503 and 505 (e.g., sys.version and sys.version_info). If sys is not imported globally, this will cause a NameError at runtime when the fallback path is executed. Given that import warnings is performed locally within this block, import sys should also be included to ensure the block is self-contained and functional. Additionally, if this change updates the minimum supported Python version, ensure the project's metadata in setup.py or pyproject.toml is also updated to reflect the same minimum version.

Suggested change
import warnings
_py_version_str = sys.version.split()[0]
import sys
import warnings
_py_version_str = sys.version.split()[0]
References
  1. When updating the minimum supported Python version in runtime checks (e.g., 'sys.version_info' checks), ensure that the project's metadata in 'setup.py' or 'pyproject.toml' is also updated to reflect the same minimum version in 'python_requires' and Trove classifiers.

Comment on lines +645 to +655
google-auth
# cryptography is a direct dependency of google-auth
cryptography
google-api-core==2.27.0
google-cloud-core==2.4.2
google-resumable-media==2.7.2
grpcio==1.33.2
requests==2.22.0
google-crc32c==1.6.0
protobuf==4.25.8
opentelemetry-api==1.1.0
google-api-core
google-cloud-core
google-resumable-media
grpcio
requests
google-crc32c
protobuf
opentelemetry-api
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 replacement for constraints-3.10.txt removes version pins for several dependencies (e.g., google-auth, google-api-core, grpcio). Constraints files are typically intended to pin specific versions to ensure reproducible test environments. Removing these pins may lead to non-deterministic behavior in CI if newer, incompatible versions are released. Please confirm if unpinning these dependencies is intentional.

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

Labels

do not merge Indicates a pull request not ready for merge, due to either quality or timing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant