-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[WIP] chore: librarian update image pull request: 20251216T210030Z #14973
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
Conversation
…prod/images-prod/python-librarian-generator@sha256:b8058df4c45e9a6e07f6b4d65b458d0d059241dd34c814f151c8bf6b89211209
|
One or more libraries have failed to generate, please review PR description for a list of failed libraries. |
Summary of ChangesHello @parthea, 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 primarily focuses on updating the core image used for generating client libraries and enhancing the robustness of the generated code. It introduces comprehensive Python version and dependency checks, refactors mTLS client certificate logic for improved clarity, and removes outdated keyword argument fixup scripts. These changes aim to ensure compatibility with newer Python versions and streamline the client library generation process, although some libraries are temporarily blocked from generation. 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
|
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.
Code Review
This pull request from the librarian tool updates the generator image and regenerates client libraries. The changes are mostly within the google-cloud-discoveryengine package and its various API versions. Key improvements include adding Python version and dependency checks in __init__.py files, and refactoring mTLS client certificate handling into a shared static method across all service clients, which reduces code duplication. Obsolete fixup scripts have been removed, and unit tests have been updated to cover the new logic.
The changes are generally good and improve the codebase. I've added a few comments on the __init__.py files regarding the use of broad except Exception: clauses in the new version-checking logic. While this is a fallback path, using more specific exceptions would improve maintainability and prevent masking unexpected errors.
| version_string: str = metadata.version(dependency_name) | ||
| parsed_version = parse_version_to_tuple(version_string) | ||
| return (parsed_version, version_string) | ||
| except Exception: |
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.
Using a broad except Exception: can mask unexpected errors. The comment mentions PackageNotFoundError, and the parsing logic can raise ValueError. It would be more robust to catch these specific exceptions, for example: except (metadata.PackageNotFoundError, ValueError):. This would make the code's intent clearer and prevent swallowing unrelated errors.
| + f"{_dependency_package}.", | ||
| FutureWarning, | ||
| ) | ||
| except Exception: |
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.
| version_string: str = metadata.version(dependency_name) | ||
| parsed_version = parse_version_to_tuple(version_string) | ||
| return (parsed_version, version_string) | ||
| except Exception: |
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.
Using a broad except Exception: can mask unexpected errors. The comment mentions PackageNotFoundError, and the parsing logic can raise ValueError. It would be more robust to catch these specific exceptions, for example: except (metadata.PackageNotFoundError, ValueError):. This would make the code's intent clearer and prevent swallowing unrelated errors.
| + f"{_dependency_package}.", | ||
| FutureWarning, | ||
| ) | ||
| except Exception: |
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.
| parsed_version = parse_version_to_tuple(version_string) | ||
| return (parsed_version, version_string) | ||
| except Exception: | ||
| # Catch exceptions from metadata.version() (e.g., PackageNotFoundError) |
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.
Using a broad except Exception: can mask unexpected errors. The comment mentions PackageNotFoundError, and the parsing logic can raise ValueError. It would be more robust to catch these specific exceptions, for example: except (metadata.PackageNotFoundError, ValueError):. This would make the code's intent clearer and prevent swallowing unrelated errors.
| FutureWarning, | ||
| ) | ||
| except Exception: | ||
| warnings.warn( |
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.
feat: update image to us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:b8058df4c45e9a6e07f6b4d65b458d0d059241dd34c814f151c8bf6b89211209
Generation failed for