-
Notifications
You must be signed in to change notification settings - Fork 83
chore: librarian generate pull request: 20251215T152333Z #1138
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
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 integrates the new Firestore Pipelines API into the 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 is an auto-generated update from protos, primarily introducing the execute_pipeline API. The changes include adding the new RPC and its related types, transport implementations, and tests. My review focuses on a performance improvement in the new client methods. I've suggested pre-compiling regular expressions at the module level to avoid repeated compilation on each method call, which will improve efficiency.
| routing_param_regex = re.compile("^projects/(?P<project_id>[^/]+)(?:/.*)?$") | ||
| regex_match = routing_param_regex.match(request.database) | ||
| if regex_match and regex_match.group("project_id"): | ||
| header_params["project_id"] = regex_match.group("project_id") | ||
|
|
||
| routing_param_regex = re.compile( | ||
| "^projects/[^/]+/databases/(?P<database_id>[^/]+)(?:/.*)?$" | ||
| ) | ||
| regex_match = routing_param_regex.match(request.database) | ||
| if regex_match and regex_match.group("database_id"): | ||
| header_params["database_id"] = regex_match.group("database_id") |
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.
For performance, it's best to compile regular expressions only once at the module level, rather than inside a function that might be called frequently. This avoids the overhead of recompiling the regex on every call.
Please define _PROJECT_ID_REGEX and _DATABASE_ID_REGEX as module-level constants. I've also used more descriptive variable names for the match objects to improve clarity and avoid reusing the regex_match variable.
| routing_param_regex = re.compile("^projects/(?P<project_id>[^/]+)(?:/.*)?$") | |
| regex_match = routing_param_regex.match(request.database) | |
| if regex_match and regex_match.group("project_id"): | |
| header_params["project_id"] = regex_match.group("project_id") | |
| routing_param_regex = re.compile( | |
| "^projects/[^/]+/databases/(?P<database_id>[^/]+)(?:/.*)?$" | |
| ) | |
| regex_match = routing_param_regex.match(request.database) | |
| if regex_match and regex_match.group("database_id"): | |
| header_params["database_id"] = regex_match.group("database_id") | |
| # For performance, compile regexes at module level and reuse them. | |
| # Using _PROJECT_ID_REGEX defined at module level. | |
| project_id_match = _PROJECT_ID_REGEX.match(request.database) | |
| if project_id_match and project_id_match.group("project_id"): | |
| header_params["project_id"] = project_id_match.group("project_id") | |
| # Using _DATABASE_ID_REGEX defined at module level. | |
| database_id_match = _DATABASE_ID_REGEX.match(request.database) | |
| if database_id_match and database_id_match.group("database_id"): | |
| header_params["database_id"] = database_id_match.group("database_id") | |
| routing_param_regex = re.compile("^projects/(?P<project_id>[^/]+)(?:/.*)?$") | ||
| regex_match = routing_param_regex.match(request.database) | ||
| if regex_match and regex_match.group("project_id"): | ||
| header_params["project_id"] = regex_match.group("project_id") | ||
|
|
||
| routing_param_regex = re.compile( | ||
| "^projects/[^/]+/databases/(?P<database_id>[^/]+)(?:/.*)?$" | ||
| ) | ||
| regex_match = routing_param_regex.match(request.database) | ||
| if regex_match and regex_match.group("database_id"): | ||
| header_params["database_id"] = regex_match.group("database_id") |
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.
For performance, it's best to compile regular expressions only once at the module level, rather than inside a function that might be called frequently. This avoids the overhead of recompiling the regex on every call.
Please define _PROJECT_ID_REGEX and _DATABASE_ID_REGEX as module-level constants. I've also used more descriptive variable names for the match objects to improve clarity and avoid reusing the regex_match variable.
| routing_param_regex = re.compile("^projects/(?P<project_id>[^/]+)(?:/.*)?$") | |
| regex_match = routing_param_regex.match(request.database) | |
| if regex_match and regex_match.group("project_id"): | |
| header_params["project_id"] = regex_match.group("project_id") | |
| routing_param_regex = re.compile( | |
| "^projects/[^/]+/databases/(?P<database_id>[^/]+)(?:/.*)?$" | |
| ) | |
| regex_match = routing_param_regex.match(request.database) | |
| if regex_match and regex_match.group("database_id"): | |
| header_params["database_id"] = regex_match.group("database_id") | |
| # For performance, compile regexes at module level and reuse them. | |
| # Using _PROJECT_ID_REGEX defined at module level. | |
| project_id_match = _PROJECT_ID_REGEX.match(request.database) | |
| if project_id_match and project_id_match.group("project_id"): | |
| header_params["project_id"] = project_id_match.group("project_id") | |
| # Using _DATABASE_ID_REGEX defined at module level. | |
| database_id_match = _DATABASE_ID_REGEX.match(request.database) | |
| if database_id_match and database_id_match.group("database_id"): | |
| header_params["database_id"] = database_id_match.group("database_id") | |
PR created by the Librarian CLI to generate Cloud Client Libraries code from protos.
BEGIN_COMMIT
BEGIN_NESTED_COMMIT
feat: publish the pipelines API to the stable branch
PiperOrigin-RevId: 840829013
Library-IDs: google-cloud-firestore
Source-link: googleapis/googleapis@534adc56
END_NESTED_COMMIT
BEGIN_NESTED_COMMIT
docs: minor api documentation changes
PiperOrigin-RevId: 840398028
Library-IDs: google-cloud-firestore
Source-link: googleapis/googleapis@bfdeefc2
END_NESTED_COMMIT
END_COMMIT
This pull request is generated with proto changes between
googleapis/googleapis@659ea6e9
(exclusive) and
googleapis/googleapis@534adc56
(inclusive).
Librarian Version: v0.7.0
Language Image: us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:ce48ed695c727f7e13efd1fd68f466a55a0d772c87b69158720cec39965bc8b2