-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(modelarmor): Added floor settings snippets #10066
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
base: main
Are you sure you want to change the base?
feat(modelarmor): Added floor settings snippets #10066
Conversation
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.
Hello @tirthrajsinh-zala-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 interacting with the Model Armor API, specifically focusing on retrieving and updating floor settings for different scopes (Project, Folder, and Organization). The changes include adding a README with prerequisites, a pom.xml file for dependencies, and several Java files containing the code snippets for getting and updating floor settings.
Highlights
- New Samples: Adds new Java code snippets demonstrating how to retrieve and update Model Armor floor settings for projects, folders, and organizations.
- README Update: Includes a README file with instructions on enabling the Model Armor API, granting necessary permissions, and setting environment variables.
- POM Configuration: Adds a pom.xml file to manage dependencies, including google-cloud-modelarmor, google-cloud-dlp, protobuf-java-util, and lombok.
Changelog
Click here to see the changelog
- modelarmor/README.md
- Added a README file explaining the Google Model Armor service.
- Provides instructions for enabling the API and granting permissions.
- Specifies the environment variables required to run the samples.
- modelarmor/pom.xml
- Added a pom.xml file to manage project dependencies.
- Includes dependencies for google-cloud-modelarmor, google-cloud-dlp, protobuf-java-util, lombok, junit, and truth.
- Configures the maven compiler plugin to use Java 11.
- modelarmor/src/main/java/modelarmor/GetFolderFloorSettings.java
- Added a sample to retrieve floor settings for a specific folder.
- Uses ModelArmorClient to get the floor setting.
- Prints the folder floor settings in JSON format.
- modelarmor/src/main/java/modelarmor/GetOrganizationFloorSettings.java
- Added a sample to retrieve floor settings for a specific organization.
- Uses ModelArmorClient to get the floor setting.
- Prints the organization floor settings in JSON format.
- modelarmor/src/main/java/modelarmor/GetProjectFloorSettings.java
- Added a sample to retrieve floor settings for a specific project.
- Uses ModelArmorClient to get the floor setting.
- Prints the project floor settings in JSON format.
- modelarmor/src/main/java/modelarmor/UpdateFolderFloorSettings.java
- Added a sample to update floor settings for a specific folder.
- Uses ModelArmorClient to update the floor setting.
- Sets RAI filter settings for dangerous and harassment filter types.
- Prints the updated folder floor settings in JSON format.
- modelarmor/src/main/java/modelarmor/UpdateOrganizationsFloorSettings.java
- Added a sample to update floor settings for a specific organization.
- Uses ModelArmorClient to update the floor setting.
- Sets RAI filter settings for dangerous and harassment filter types.
- Prints the updated organization floor settings in JSON format.
- modelarmor/src/main/java/modelarmor/UpdateProjectFloorSettings.java
- Added a sample to update floor settings for a specific project.
- Uses ModelArmorClient to update the floor setting.
- Sets RAI filter settings for dangerous and harassment filter types.
- Prints the updated project floor settings in JSON format.
- modelarmor/src/test/java/modelarmor/SnippetsIT.java
- Added integration tests for the new samples.
- Tests retrieval and update of floor settings for project, folder and organization.
- Uses JUnit and Truth for testing.
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 realms of code, where models reside,
Armor we forge, with settings to confide.
Project, folder, organization's grace,
Guarding AI's evolving space.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The pull request adds samples for retrieving and updating Model Armor floor settings at the project, folder, and organization levels. The code is well-structured and easy to follow. However, there are a few areas that could be improved for clarity and robustness.
Summary of Findings
- Missing Input Validation: The code lacks input validation for the folderId, organizationId, and projectId variables. This could lead to unexpected behavior or errors if these variables are not properly set. Consider adding checks to ensure these variables are not null or empty before using them.
- Hardcoded Location: The LOCATION variable is hardcoded to "us-central1" in the SnippetsIT.java file. This may not be appropriate for all users. Consider making this configurable or removing it entirely.
- Missing Error Handling: The code does not handle potential exceptions that may occur during API calls. This could lead to the application crashing or behaving unexpectedly. Consider adding try-catch blocks to handle these exceptions and provide informative error messages.
Merge Readiness
The pull request is almost ready for merging. However, I recommend addressing the issues mentioned in the review comments before merging. I am unable to directly approve the pull request, and users should have others review and approve this code before merging. Addressing the input validation and error handling issues will improve the robustness and reliability of the code.
modelarmor/src/main/java/modelarmor/GetFolderFloorSettings.java
Outdated
Show resolved
Hide resolved
modelarmor/src/main/java/modelarmor/GetFolderFloorSettings.java
Outdated
Show resolved
Hide resolved
modelarmor/src/main/java/modelarmor/GetOrganizationFloorSettings.java
Outdated
Show resolved
Hide resolved
modelarmor/src/main/java/modelarmor/GetOrganizationFloorSettings.java
Outdated
Show resolved
Hide resolved
modelarmor/src/main/java/modelarmor/GetProjectFloorSettings.java
Outdated
Show resolved
Hide resolved
modelarmor/src/main/java/modelarmor/UpdateOrganizationsFloorSettings.java
Outdated
Show resolved
Hide resolved
modelarmor/src/main/java/modelarmor/UpdateProjectFloorSettings.java
Outdated
Show resolved
Hide resolved
modelarmor/src/main/java/modelarmor/UpdateProjectFloorSettings.java
Outdated
Show resolved
Hide resolved
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.
This PR has the same global issues as #10068 .
Here is the summary of changes. You are about to add 6 region tags.
This comment is generated by snippet-bot.
|
@telpirion Thanks for the review! All global issues have been addressed in this PR. For tests to pass, could you please help us to set environment variables as mentioned in the PR description? |
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.
This is very close to being ready to merge--thank you! Just has two blocking comments on the tests.
|
||
@Before | ||
public void beforeEach() { | ||
stdOut = new ByteArrayOutputStream(); |
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.
issue(blocking): be sure to store a reference to the pipe assigned to stdout before you assign the FD to something else.
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.
Done. Please verify. If it looks good, I'll update other PRs as well. Thanks!
|
||
@After | ||
public void afterEach() throws IOException { | ||
stdOut = null; |
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.
issue(blocking): restore the stdout FD so that it points to the default pipe.
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.
Done. Please verify. If it looks good, I'll update other PRs as well. Thanks!
Description
Added snippets to interact with Model Armor floor settings.
Checklist
pom.xml
parent set to latestshared-configuration
(MA_FOLDER_ID, MA_ORG_ID), Please set the values by referring to https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/model_armor/snippets/noxfile_config.py#L42-L43
mvn clean verify
requiredmvn -P lint checkstyle:check
requiredmvn -P lint clean compile pmd:cpd-check spotbugs:check
advisory only