docs(samples): add template/monitoring samples #174

FrodoTheTrue · 2022-01-26T08:53:27Z

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

snippet-bot · 2022-01-26T08:53:34Z

Here is the summary of changes.

You are about to add 7 region tags.

samples/snippets/create_certificate_template.py:17, tag privateca_create_certificate_template
samples/snippets/delete_certificate_template.py:17, tag privateca_delete_certificate_template
samples/snippets/list_certificate_templates.py:17, tag privateca_list_certificate_template
samples/snippets/monitor_certificate_authority.py:17, tag privateca_monitor_ca_expiry
samples/snippets/update_ca_pool_issuance_policy.py:17, tag privateca_set_issuance_policy
samples/snippets/update_certificate_authority.py:17, tag privateca_update_ca_label
samples/snippets/update_certificate_template.py:17, tag privateca_update_certificate_template

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

leahecole · 2022-01-27T22:21:09Z

Hey @savijatv! Sorry I haven't sent you any reviews recently - can you PTAL at this and let me know when you've reviewed it?

savijatv · 2022-01-28T07:32:20Z

samples/snippets/monitor_certificate_authority.py

+    Create a monitoring policy that notifies you 30 days before a managed CA expires.
+
+    Args:
+        project_id: project ID or project number of the Cloud project you want to use.


IIUC, project ID and project number are different entities. Does this function accept either or these? If not, please specify the correct one, project ID OR project number.

great catch - they are different entities

For python-compute we use the same description: https://github.com/googleapis/python-compute/blob/main/samples/snippets/sample_create_vm.py#L167

Here we accept both entities as an input parameter: project ID and project number

@leahecole @savijatv WDYT?

I still think that they're different. The Project ID starts with a letter whereas Project number is all numeric. See https://cloud.google.com/resource-manager/docs/creating-managing-projects.
Does the code accept both?

Yep, have just checked, the code accepts both

savijatv · 2022-01-28T07:33:26Z

samples/snippets/list_certificate_templates.py

+    List the certificate templates present in the given project and location.
+
+    Args:
+        project_id: project ID or project number of the Cloud project you want to use.


IIUC, project ID and project number are different entities. Does this function accept either or these? If not, please specify the correct one, project ID OR project number.

great catch - they are different entities

savijatv · 2022-01-28T07:33:34Z

samples/snippets/delete_certificate_template.py

+    Delete the certificate template present in the given project and location.
+
+    Args:
+        project_id: project ID or project number of the Cloud project you want to use.


IIUC, project ID and project number are different entities. Does this function accept either or these? If not, please specify the correct one, project ID OR project number.

great catch - they are different entities

I have double checked with a gloud projects describe and it seems that both project ID and number work in API calls. See more here: https://cloud.google.com/resource-manager/docs/creating-managing-projects

To be on the safe side we should test this sample with ID and number as well.

Agreed that this is a bit strange behavior
Makes sense to add a test.

samples/snippets/delete_certificate_template.py

samples/snippets/create_certificate_template.py

samples/snippets/update_certificate_template.py

samples/snippets/update_ca_pool_issuance_policy.py

savijatv · 2022-01-28T11:48:50Z

samples/snippets/monitor_certificate_authority.py

+        alert_policy=alert_policy,
+    )
+
+    print("Monitoring policy successfully created !", policy.name)


Remove space before !

Also, in case of any error, does this sample print an error message?

There is a good question. But this PR is just a python equivalent of Java samples: https://github.com/googleapis/java-security-private-ca/blob/main/samples/snippets/cloud-client/src/main/java/privateca/MonitorCertificateAuthority.java#L87

We can fix it later I guess

samples/snippets/monitor_certificate_authority.py

savijatv · 2022-01-28T11:59:23Z

samples/snippets/list_certificate_templates.py

+    request = privateca_v1.ListCertificateTemplatesRequest(
+        parent=caServiceClient.common_location_path(project_id, location),
+    )
+


Aren't we supposed to print an error message, in case of an error?

I guess here we have either exception itself or printed list of templates

savijatv

If the code accepts both project ID and project number as the argument, please ignore my comment.

product-auto-label bot added api: security-privateca Issues related to the googleapis/python-security-private-ca API. samples Issues that are directly related to samples. labels Jan 26, 2022

FrodoTheTrue requested a review from leahecole January 26, 2022 13:46

savijatv reviewed Jan 28, 2022

View reviewed changes

parthea assigned FrodoTheTrue Feb 3, 2022

parthea changed the title ~~samples(security): add template/monitoring samples~~ docs(samples): add template/monitoring samples Feb 3, 2022

FrodoTheTrue requested a review from a team as a code owner February 11, 2022 12:46

savijatv approved these changes Feb 11, 2022

View reviewed changes

leahecole approved these changes Feb 11, 2022

View reviewed changes

FrodoTheTrue added 2 commits February 14, 2022 13:33

samples(security): add template/monitoring samples

2af7b56

review fixes

25d0cb5

FrodoTheTrue force-pushed the ca-templates-samples branch from 70e0749 to 25d0cb5 Compare February 14, 2022 13:33

FrodoTheTrue merged commit 9fbd3e1 into main Feb 14, 2022

FrodoTheTrue deleted the ca-templates-samples branch February 14, 2022 13:41

release-please bot mentioned this pull request Feb 14, 2022

chore(main): release 1.2.3 #181

Merged

docs(samples): add template/monitoring samples #174

docs(samples): add template/monitoring samples #174

Uh oh!

Conversation

FrodoTheTrue commented Jan 26, 2022

Uh oh!

snippet-bot bot commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

leahecole commented Jan 27, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

savijatv left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

snippet-bot bot commented Jan 26, 2022 •

edited

Loading