feat(parametermanager): Added samples to view, list and render parameter in specified region #10051
Conversation
vatsal-vora-crestdata
requested review from
yoshi-approver and
a team
as code owners
product-auto-label
bot
added
the
samples
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @vatsal-vora-crestdata, 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 introduces samples for interacting with the Parameter Manager SDK, specifically focusing on viewing, listing, and rendering parameters and their versions within specified regions. It includes samples for listing all parameters, viewing parameter details, listing parameter versions, viewing parameter version details, and rendering secret parameter versions. The PR also includes corresponding tests for the new samples.
Highlights
- New samples: Added five new samples demonstrating how to view, list, and render parameters and parameter versions in a specified region using the Parameter Manager SDK.
- Regional focus: The samples are designed to work with regional parameters, ensuring that the code interacts with the Parameter Manager service in a specific location.
- Testing: Included tests for the new samples to ensure they function correctly and as expected.
Changelog
Click here to see the changelog
- parametermanager/pom.xml
- Added dependencies for google-cloud-parametermanager, protobuf-java-util, and lombok.
- Added test dependencies for junit, google-truth, google-cloud-secretmanager, and google-iam-policy.
- Configured the maven-compiler-plugin to use source and target version 11, and added lombok as an annotation processor.
- parametermanager/src/main/java/parametermanager/regionalsamples/GetRegionalParam.java
- Created a new sample to get a regional parameter using the Parameter Manager SDK.
- Demonstrates how to initialize the ParameterManagerClient with a regional endpoint.
- Shows how to build the parameter name and retrieve the parameter details.
- parametermanager/src/main/java/parametermanager/regionalsamples/GetRegionalParamVersion.java
- Created a new sample to get a regional parameter version using the Parameter Manager SDK.
- Demonstrates how to initialize the ParameterManagerClient with a regional endpoint.
- Shows how to build the parameter version name and retrieve the parameter version details.
- parametermanager/src/main/java/parametermanager/regionalsamples/ListRegionalParamVersions.java
- Created a new sample to list parameter versions for a specified region using the Parameter Manager SDK.
- Demonstrates how to initialize the ParameterManagerClient with a regional endpoint.
- Shows how to build the parameter name and list all parameter versions.
- parametermanager/src/main/java/parametermanager/regionalsamples/ListRegionalParams.java
- Created a new sample to list parameters for a specified region using the Parameter Manager SDK.
- Demonstrates how to initialize the ParameterManagerClient with a regional endpoint.
- Shows how to build the location name and list all parameters in the region.
- parametermanager/src/main/java/parametermanager/regionalsamples/RenderRegionalParamVersion.java
- Created a new sample to render a regional parameter version using the Parameter Manager SDK.
- Demonstrates how to initialize the ParameterManagerClient with a regional endpoint.
- Shows how to build the parameter version name and render the parameter version.
- parametermanager/src/test/java/parametermanager/regionalsamples/SnippetsIT.java
- Added integration tests for all the new samples.
- Includes setup and teardown methods to create and delete parameters and parameter versions for testing.
- Tests cover getting, listing, and rendering parameters and parameter versions.
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.
Did you know?
The concept of "parameters" in programming dates back to Ada Lovelace's notes on the Analytical Engine in the 1840s, where she described how variables could be passed to the engine to perform different calculations.
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 introduces samples for viewing, listing, and rendering parameters and parameter versions using the Parameter Manager SDK. The samples cover various functionalities and include corresponding tests. Overall, the code is well-structured and follows a consistent pattern. However, there are a few areas that could be improved for clarity and efficiency.
Summary of Findings
- Missing Error Handling: The code lacks proper error handling for API calls. If the API calls fail, the program might crash or behave unexpectedly. It's crucial to implement error handling to ensure the program's stability and provide informative error messages.
- Hardcoded API Endpoint: The API endpoint is hardcoded in the code. This makes it difficult to change the endpoint without modifying the code. It's better to use a configuration file or environment variable to store the API endpoint.
- Inconsistent use of try-with-resources: The code uses try-with-resources for
ParameterManagerClientandSecretManagerServiceClientin some places but not others. It's important to use try-with-resources consistently to ensure that resources are properly closed.
Merge Readiness
The pull request is not quite ready to be merged. While the code is functional, the lack of error handling and the hardcoded API endpoint are significant concerns that need to be addressed before merging. I am unable to approve this pull request, and recommend that the author address the issues raised in the review comments before merging. It would also be beneficial to add more comments to the code to improve its readability.
parametermanager/src/main/java/parametermanager/regionalsamples/GetRegionalParam.java
Show resolved
Hide resolved
parametermanager/src/main/java/parametermanager/regionalsamples/GetRegionalParamVersion.java
Show resolved
Hide resolved
parametermanager/src/main/java/parametermanager/regionalsamples/ListRegionalParamVersions.java
Show resolved
Hide resolved
parametermanager/src/main/java/parametermanager/regionalsamples/ListRegionalParams.java
Show resolved
Hide resolved
parametermanager/src/main/java/parametermanager/regionalsamples/RenderRegionalParamVersion.java
Show resolved
Hide resolved
parametermanager/src/main/java/parametermanager/regionalsamples/GetRegionalParam.java
Show resolved
Hide resolved
| try (SecretManagerServiceClient client = | ||
| SecretManagerServiceClient.create(secretManagerServiceSettings)) { | ||
| return client.createSecret(locationName.toString(), secretId, secret); | ||
| } |
There was a problem hiding this comment.
It's good practice to use try-with-resources to ensure that the SecretManagerServiceClient is closed properly. This helps prevent resource leaks.
try (
SecretManagerServiceClient client =
SecretManagerServiceClient.create(secretManagerServiceSettings)) {
return client.createSecret(locationName.toString(), secretId, secret);
}| try (SecretManagerServiceClient client = | ||
| SecretManagerServiceClient.create(secretManagerServiceSettings)) { | ||
| client.addSecretVersion(request); | ||
| } |
There was a problem hiding this comment.
It's good practice to use try-with-resources to ensure that the SecretManagerServiceClient is closed properly. This helps prevent resource leaks.
try (
SecretManagerServiceClient client =
SecretManagerServiceClient.create(secretManagerServiceSettings)) {
client.addSecretVersion(request);
}| try (SecretManagerServiceClient client = | ||
| SecretManagerServiceClient.create(secretManagerServiceSettings)) { | ||
| client.deleteSecret(name); | ||
| } catch (com.google.api.gax.rpc.NotFoundException e) { |
There was a problem hiding this comment.
It's good practice to use try-with-resources to ensure that the SecretManagerServiceClient is closed properly. This helps prevent resource leaks.
try (
SecretManagerServiceClient client =
SecretManagerServiceClient.create(secretManagerServiceSettings)) {
client.deleteSecret(name);
}| try (SecretManagerServiceClient client = | ||
| SecretManagerServiceClient.create(secretManagerServiceSettings)) { |
There was a problem hiding this comment.
It's good practice to use try-with-resources to ensure that the SecretManagerServiceClient is closed properly. This helps prevent resource leaks.
try (
SecretManagerServiceClient client =
SecretManagerServiceClient.create(secretManagerServiceSettings)) {
Policy currentPolicy =
client.getIamPolicy(
GetIamPolicyRequest.newBuilder().setResource(secretName.toString()).build());
There was a problem hiding this comment.
As with other parameter manager PRs, the samples themselves look really good -- no changes needed. We just need to ensure that we're using client libraries directly for resource creation and teardown in tests. I recommend creating a Utils class in the test/ package that does common creation & teardown.
|
|
||
| // test create parameter version with unformatted format | ||
| TEST_PARAMETER_NAME_FOR_VERSION = ParameterName.of(PROJECT_ID, LOCATION_ID, randomId()); | ||
| createParameter(TEST_PARAMETER_NAME_FOR_VERSION.getParameter(), ParameterFormat.UNFORMATTED); |
There was a problem hiding this comment.
issue(blocking): do not use samples for set up and tear down of resources in tests.
There was a problem hiding this comment.
Not using samples for setup and teardown of resources in tests. Already created methods in the test file itself for createParameter and deleteParameter, same for others.
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-configurationGOOGLE_CLOUD_PROJECTandGOOGLE_CLOUD_PROJECT_LOCATIONto be set.mvn clean verifyrequiredmvn -P lint checkstyle:checkrequiredmvn -P lint clean compile pmd:cpd-check spotbugs:checkadvisory only