feat(parametermanager): Added samples for get, list and render regional parameter & parameter version #10061
Conversation
durgesh-ninave-crest
requested review from
yoshi-approver and
a team
as code owners
product-auto-label
bot
added
the
samples
![gemini-code-assist[bot]](https://flashblogg.com:443/je/cache.php/https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @durgesh-ninave-crest, 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!
Summary of Changes
This pull request adds samples for viewing, listing, and rendering parameter and parameter versions using the Parameter Manager SDK. It includes samples for listing all parameters, viewing parameter details, listing all parameter versions, viewing parameter version details, and rendering a secret parameter version. The pull request also includes the required tests for the new samples.
Highlights
- New samples: Added samples for listing, viewing, and rendering regional parameters and their versions using the Parameter Manager SDK.
- Regional Parameter Manager: The samples use the regional endpoint for the Parameter Manager API, ensuring that the operations are performed in the specified region.
- Testing: Added new tests to verify the functionality of the new samples.
Changelog
Click here to see the changelog
- parametermanager/src/main/java/parametermanager/regionalsamples/GetRegionalParam.java
- Added a new class to demonstrate how to get a regional parameter using the Parameter Manager SDK.
- Includes a method to get a regional parameter by project ID, location ID, and parameter ID.
- Uses the ParameterManagerClient to interact with the Parameter Manager API.
- parametermanager/src/main/java/parametermanager/regionalsamples/GetRegionalParamVersion.java
- Added a new class to demonstrate how to get a regional parameter version using the Parameter Manager SDK.
- Includes a method to get a regional parameter version by project ID, location ID, parameter ID, and version ID.
- Uses the ParameterManagerClient to interact with the Parameter Manager API.
- parametermanager/src/main/java/parametermanager/regionalsamples/ListRegionalParamVersions.java
- Added a new class to list parameter versions for a specified region using the Parameter Manager SDK.
- Includes a method to list all parameter versions regionally by project ID, location ID, and parameter ID.
- Uses the ParameterManagerClient to interact with the Parameter Manager API.
- parametermanager/src/main/java/parametermanager/regionalsamples/ListRegionalParams.java
- Added a new class to demonstrate listing parameters for a specified region using the Parameter Manager SDK.
- Includes a method to list all parameters in a given region by project ID and location ID.
- Uses the ParameterManagerClient to interact with the Parameter Manager API.
- parametermanager/src/main/java/parametermanager/regionalsamples/RenderRegionalParamVersion.java
- Added a new class to demonstrate how to render a regional parameter version using the Parameter Manager SDK.
- Includes a method to render a regional parameter version by project ID, location ID, parameter ID, and version ID.
- Uses the ParameterManagerClient to interact with the Parameter Manager API.
- parametermanager/src/test/java/parametermanager/regionalsamples/SnippetsIT.java
- Added tests for the new samples, including tests for getting, listing, and rendering regional parameters and their versions.
- Includes setup and teardown methods to create and delete test resources.
- Added methods to create and manage secrets for testing the render parameter version functionality (lines 291-370).
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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.
In regions code takes flight,
Parameters managed, day and night,
Versions render,
Secrets surrender,
SDK samples, shining bright.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request adds samples for viewing, listing, and rendering regional parameters and parameter versions using the Parameter Manager SDK. The changes include new classes for each sample and corresponding tests. Overall, the code is well-structured and follows the sample format guide. However, there are a few areas that could be improved for clarity and efficiency.
Summary of Findings
- TODO comments: The code contains
TODOcomments that should be replaced with actual values before merging. These comments indicate areas where developers need to provide specific project and parameter information. - Redundant endpoint configuration: The endpoint configuration is repeated in multiple files. Consider creating a utility method or constant to avoid duplication and improve maintainability.
- Thread.sleep usage: The use of
Thread.sleepin the test setup is not ideal for integration tests. Explore alternative approaches for ensuring resources are available before proceeding with tests.
Merge Readiness
The pull request is not quite ready for merging. The TODO comments need to be addressed, and the redundant endpoint configuration should be refactored. The Thread.sleep usage in the tests should also be reviewed and potentially replaced with a more robust solution. I am unable to approve this pull request, and recommend that others review and approve this code before merging.
| // TODO(developer): Replace these variables before running the sample. | ||
| String projectId = "your-project-id"; | ||
| String locationId = "your-location-id"; | ||
| String parameterId = "your-parameter-id"; |
There was a problem hiding this comment.
These TODO comments should be replaced with actual values before merging. Consider adding a comment explaining the purpose of each variable.
| // TODO(developer): Replace these variables before running the sample. | |
| String projectId = "your-project-id"; | |
| String locationId = "your-location-id"; | |
| String parameterId = "your-parameter-id"; | |
| // TODO(developer): Replace these variables before running the sample. | |
| String projectId = "your-project-id"; // Project to create parameter in. | |
| String locationId = "your-location-id"; // Location of the parameter. | |
| String parameterId = "your-parameter-id"; // ID of the parameter. |
| // TODO(developer): Replace these variables before running the sample. | ||
| String projectId = "your-project-id"; | ||
| String locationId = "your-location-id"; | ||
| String parameterId = "your-parameter-id"; | ||
| String versionId = "your-version-id"; |
There was a problem hiding this comment.
These TODO comments should be replaced with actual values before merging. Consider adding a comment explaining the purpose of each variable.
| // TODO(developer): Replace these variables before running the sample. | |
| String projectId = "your-project-id"; | |
| String locationId = "your-location-id"; | |
| String parameterId = "your-parameter-id"; | |
| String versionId = "your-version-id"; | |
| // TODO(developer): Replace these variables before running the sample. | |
| String projectId = "your-project-id"; // Project to create parameter in. | |
| String locationId = "your-location-id"; // Location of the parameter. | |
| String parameterId = "your-parameter-id"; // ID of the parameter. | |
| String versionId = "your-version-id"; // Version of the parameter. |
| // TODO(developer): Replace these variables before running the sample. | ||
| String projectId = "your-project-id"; | ||
| String locationId = "your-location-id"; | ||
| String parameterId = "your-parameter-id"; |
There was a problem hiding this comment.
These TODO comments should be replaced with actual values before merging. Consider adding a comment explaining the purpose of each variable.
| // TODO(developer): Replace these variables before running the sample. | |
| String projectId = "your-project-id"; | |
| String locationId = "your-location-id"; | |
| String parameterId = "your-parameter-id"; | |
| // TODO(developer): Replace these variables before running the sample. | |
| String projectId = "your-project-id"; // Project to create parameter in. | |
| String locationId = "your-location-id"; // Location of the parameter. | |
| String parameterId = "your-parameter-id"; // ID of the parameter. |
| // TODO(developer): Replace these variables before running the sample. | ||
| String projectId = "your-project-id"; | ||
| String locationId = "your-location-id"; |
There was a problem hiding this comment.
These TODO comments should be replaced with actual values before merging. Consider adding a comment explaining the purpose of each variable.
| // TODO(developer): Replace these variables before running the sample. | |
| String projectId = "your-project-id"; | |
| String locationId = "your-location-id"; | |
| // TODO(developer): Replace these variables before running the sample. | |
| String projectId = "your-project-id"; // Project to create parameter in. | |
| String locationId = "your-location-id"; // Location of the parameter. |
| // TODO(developer): Replace these variables before running the sample. | ||
| String projectId = "your-project-id"; | ||
| String locationId = "your-location-id"; | ||
| String parameterId = "your-parameter-id"; | ||
| String versionId = "your-version-id"; |
There was a problem hiding this comment.
These TODO comments should be replaced with actual values before merging. Consider adding a comment explaining the purpose of each variable.
| // TODO(developer): Replace these variables before running the sample. | |
| String projectId = "your-project-id"; | |
| String locationId = "your-location-id"; | |
| String parameterId = "your-parameter-id"; | |
| String versionId = "your-version-id"; | |
| // TODO(developer): Replace these variables before running the sample. | |
| String projectId = "your-project-id"; // Project to create parameter in. | |
| String locationId = "your-location-id"; // Location of the parameter. | |
| String parameterId = "your-parameter-id"; // ID of the parameter. | |
| String versionId = "your-version-id"; // Version of the parameter. |
| // Endpoint to call the regional parameter manager server | ||
| String apiEndpoint = String.format("parametermanager.%s.rep.googleapis.com:443", locationId); | ||
| ParameterManagerSettings parameterManagerSettings = | ||
| ParameterManagerSettings.newBuilder().setEndpoint(apiEndpoint).build(); |
There was a problem hiding this comment.
The endpoint configuration is repeated in multiple files. Consider creating a utility method or constant to avoid duplication and improve maintainability.
// Endpoint to call the regional parameter manager server
String apiEndpoint = String.format("parametermanager.%s.rep.googleapis.com:443", locationId);
ParameterManagerSettings parameterManagerSettings = ParameterManagerSettings.newBuilder().setEndpoint(apiEndpoint).build();
//Consider refactoring to:
//private static final String API_ENDPOINT_FORMAT = "parametermanager.%s.rep.googleapis.com:443";
//String apiEndpoint = String.format(API_ENDPOINT_FORMAT, locationId);
//ParameterManagerSettings parameterManagerSettings = ParameterManagerSettings.newBuilder().setEndpoint(apiEndpoint).build();| try { | ||
| Thread.sleep(60000); | ||
| } catch (InterruptedException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
The use of Thread.sleep is generally discouraged in integration tests as it can lead to flaky tests and increased test execution time. Consider using a more robust approach, such as polling or waiting for a specific condition to be met.
try {
Thread.sleep(60000);
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
//Consider refactoring to:
//long startTime = System.currentTimeMillis();
//while (!isSecretAvailable(SECRET_NAME) && System.currentTimeMillis() - startTime < 60000) {
// Thread.sleep(1000); // Poll every 1 second
//}
//if (!isSecretAvailable(SECRET_NAME)) {
// throw new RuntimeException("Secret not available after 60 seconds");
//}
gptSanyam
added
kokoro:run
kokoro-team
removed
kokoro:run
Description
Added samples for viewing, listing and rendering parameter and parameter versions using Parameter Manager SDK
Sample List (regional):
Also added required Tests for the same.
Checklist
pom.xmlparent set to latestshared-configurationmvn clean verifyrequiredmvn -P lint checkstyle:checkrequiredmvn -P lint clean compile pmd:cpd-check spotbugs:checkadvisory only