feat: Automatically add ghpc_creator label to expanded blueprint#5468
Conversation
Summary of ChangesHello, 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 enhances 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request implements automatic detection of the deployment creator's username and adds a ghpc_creator label to the expanded blueprint. The username is detected by querying gcloud, checking environment variables, or using the current system user. A new CLI flag --add-creator-label is introduced to control this behavior, defaulting to true for @google.com accounts. Review feedback recommends implementing a timeout for the synchronous gcloud command execution and optimizing the code to skip username detection when the creator label is explicitly disabled via the CLI flag.
96cab26 to
29f98be
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature to automatically add a ghpc_creator label to resources, based on the user who initiated the deployment. Key changes include adding AddCreatorLabel and CreatorUsername fields to the Blueprint struct, implementing a detectUsername function, and introducing a --add-creator-label flag for create and expand commands. The flag defaults to true for @google.com users if not explicitly set. The expandGlobalLabels function now includes the ghpc_creator label with basic sanitization, and new unit tests have been added for this functionality. Feedback from the review indicates that the label sanitization needs to be more robust to meet GCP label requirements (e.g., lowercasing, character limits, and handling more invalid characters) to prevent deployment failures. Additionally, the logic for determining AddCreatorLabel could be refactored for improved readability, and the unit tests should be expanded to cover more edge cases for username sanitization.
29f98be to
c1d6009
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to tag expanded blueprints with a "ghpc_creator" label, identifying the user responsible for the deployment. It includes logic to detect the username via gcloud, environment variables, or system calls, and automatically enables this label for @google.com users. The label is sanitized to ensure compatibility with cloud provider constraints. Feedback focuses on improving the CLI's responsiveness by using command contexts for sub-processes and optimizing performance by pre-compiling regular expressions.
c1d6009 to
8b6f2ce
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a feature to automatically add a "ghpc_creator" label to expanded blueprints, defaulting to true for users with "@google.com" accounts. It includes logic to detect the username via gcloud, environment variables, or system user info, along with sanitization to ensure the label value is valid for GCP. Review feedback highlights potential performance bottlenecks caused by executing external gcloud processes on every command run. Suggestions include checking for the CLOUDSDK_CORE_ACCOUNT environment variable first to avoid unnecessary process execution and optimizing the gcloud command used for detection.
Adds --add-creator-label flag to gcluster expand/create/deploy. Detects username via gcloud auth list or shell environment. Defaults to true for @google.com accounts.
8b6f2ce to
8e7bfbe
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to automatically add a ghpc_creator label to expanded blueprints. This involves adding a new --add-creator-label flag to the create and expand commands, implementing a detectUsername function to determine the user's identity with a fallback mechanism, and sanitizing the username to comply with GCP label naming conventions. The Blueprint struct has been updated with AddCreatorLabel and CreatorUsername fields, and new unit tests have been added for the label expansion logic. Feedback suggests adding comments to the detectUsername function to explain the order of precedence for username detection and to the validLabelValueRegex to clarify its purpose and reference GCP label naming conventions, both to improve code readability and maintainability.
Adds --add-creator-label flag to gcluster expand/create/deploy.
Detects username via gcloud auth list or shell environment.
Defaults to true for @google.com accounts.
Submission Checklist
NOTE: Community submissions can take up to 2 weeks to be reviewed.
Please take the following actions before submitting this pull request.