java-docs-samples
java-docs-samples copied to clipboard
feat(scc): add v2 samples - muteconfig and findings
Add new samples for Security Command Center v2 APIs.
Total: 12 samples (MuteConfig + Findings)
Blocked by library release.
QQ: we discussed to temporarily suspend adding new samples to the repo. What is necessity for these particular samples?
@minherz Ack, but the samples are supporting a new launch. Hence, the priority in adding them.
Here is the summary of changes.
You are about to add 12 region tags.
- security-command-center/snippets/src/main/java/vtwo/findings/GroupFindings.java:19, tag
securitycenter_group_all_findings_v2 - security-command-center/snippets/src/main/java/vtwo/findings/GroupFindingsWithFilter.java:19, tag
securitycenter_group_filtered_findings_v2 - security-command-center/snippets/src/main/java/vtwo/findings/ListAllFindings.java:19, tag
securitycenter_list_all_findings_v2 - security-command-center/snippets/src/main/java/vtwo/findings/ListFindingsWithFilter.java:19, tag
securitycenter_list_filtered_findings_v2 - security-command-center/snippets/src/main/java/vtwo/muteconfig/BulkMuteFindings.java:19, tag
securitycenter_bulk_mute_v2 - security-command-center/snippets/src/main/java/vtwo/muteconfig/CreateMuteRule.java:19, tag
securitycenter_create_mute_config_v2 - security-command-center/snippets/src/main/java/vtwo/muteconfig/DeleteMuteRule.java:19, tag
securitycenter_delete_mute_config_v2 - security-command-center/snippets/src/main/java/vtwo/muteconfig/GetMuteRule.java:19, tag
securitycenter_get_mute_config_v2 - security-command-center/snippets/src/main/java/vtwo/muteconfig/ListMuteRules.java:19, tag
securitycenter_list_mute_configs_v2 - security-command-center/snippets/src/main/java/vtwo/muteconfig/SetMuteFinding.java:19, tag
securitycenter_set_mute_v2 - security-command-center/snippets/src/main/java/vtwo/muteconfig/SetUnmuteFinding.java:19, tag
securitycenter_set_unmute_v2 - security-command-center/snippets/src/main/java/vtwo/muteconfig/UpdateMuteRule.java:19, tag
securitycenter_update_mute_config_v2
This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:
- [ ] Refresh this comment
nit: it is generally recommended to narrow the scope of code snippets by placing the region tags around the actual code while omitting
main()methods and other not relevant details. Mind that you can use more the region tag in more than place within the file so you can provide necessaryimportstatements to ensure that copy-n-paste of the code snippet will compile.nit: The best practice is considered to return actual values for code snippets that get information. It allows to write more robust unit tests.
nit: It would be a good practice either to catch the API related exception or explicitly declare them (using
throwsstatement in the method declaration)Inconsistency in the error handling; there are several considerations regarding it:
- Best practices for error handling in Java
- Expose what kind of thrown exceptions are related to this particular API or user scenarion
- Support writing unit tests that validate specific errors (thrown exceptions)
Thanks for your review, Leonid!
-
This guideline is not standard and different languages have their own implementation. In Java, we have primarily resorted to following "one snippet per class" rule. Will follow up on this discussion internally to see if it's time to update this!
-
I've updated snippets that can benefit from returning results. Few snippets like listing and grouping findings, I've left the tests unchanged, primarily because this could mean passing unknown number of result records back to tests.
-
For exceptions, I've updated the snippets to follow the guideline of catching only API related exceptions.
... Thanks for your review, Leonid!
- This guideline is not standard and different languages have their own implementation. In Java, we have primarily resorted to following "one snippet per class" rule. Will follow up on this discussion internally to see if it's time to update this!
I do not follow the reference about different languages here. The first bullet in the original comment explained that the code snippet (part of code enclosed by the region tag) should show only the relevant code to ensure that the documentation is not overloaded with extra content. The files in this PR use region tags in a way that code snippet includes the whole file with the exception of the copyright comment. It is not a blocker, but including the main() method into code snippet does not contribute to the example of the API call usage but only helps to run the code sample if user decides to.
- I've updated snippets that can benefit from returning results. Few snippets like listing and grouping findings, I've left the tests unchanged, primarily because this could mean passing unknown number of result records back to tests.
It looks like only one "get" code samples was modified. It is not a blocker, but I would recommend considering to modify all "get" code samples. The delete/set/update code samples can communicate the status as not throwing exception ==> OK. If any of the delete/set/update code samples receive the status of the operation success on response, they can return that status as well. There should be no concern regarding unknown number of results because the tests should validate the expected content. Currently they do not validate a number of results or all returned content either.
- For exceptions, I've updated the snippets to follow the guideline of catching only API related exceptions.
I do not see any catch statements in the files. Is it supposed to be like this?