feat: Adding code samples and tests for them #55

m-strzelczyk · 2021-05-19T14:24:19Z

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)

Hi, I tried uploading those samples to the python-docs-samples repo, but I was told that it'd be better to put them here. So here I am, with the first batch of code samples for our documentation :)

snippet-bot · 2021-05-19T14:24:24Z

Here is the summary of changes.

You are about to add 10 region tags.

samples/snippets/quickstart.py:22, tag compute_instances_create
samples/snippets/quickstart.py:23, tag compute_instances_delete
samples/snippets/quickstart.py:26, tag compute_instances_list
samples/snippets/quickstart.py:27, tag compute_instances_list_all
samples/snippets/quickstart.py:28, tag compute_instances_operation_check
samples/snippets/quickstart.py:40, tag compute_instances_list
samples/snippets/quickstart.py:65, tag compute_instances_list_all
samples/snippets/quickstart.py:95, tag compute_instances_create
samples/snippets/quickstart.py:180, tag compute_instances_delete
samples/snippets/quickstart.py:212, tag compute_instances_operation_check

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

busunkim96

I've opened cl/374662458 to enable samples CI.

.github/CODEOWNERS

samples/snippets/quickstart.py

samples/snippets/requirements-test.txt

samples/snippets/quickstart.py

busunkim96 · 2021-05-19T16:49:46Z

samples/snippets/quickstart.py

+    print(f"Creating the {machine_name} instance in {zone}...")
+    operation = instance_client.insert(request=request)
+    # wait_result = operation_client.wait(operation=operation.name, zone=zone, project=project)
+    operation = wait_for_operation(operation, project)


The LRO experience looks different from existing GAPICs. @vam-google Does the Compute API have a non-standard LRO representation? It doesn't look like the generator produced the usual google.api_core.Operation type.

For comparison, see videointelligence where you can do operation.result(timeout=90) to poll for the result.

sample

method documentation

google.api_core.operation.Operation

@busunkim96 would a simpler approach suggest by @bshaffer work here?

Please refer to https://github.com/GoogleCloudPlatform/php-docs-samples/blob/80801c983457f407cb9a81d3b60654369f090e03/compute/cloud-client/instances/src/create_instance.php#L80

This is how I saw the team use the operations in their test code. It is an instance of google.cloud.compute_v1.types.Operation and not google.api_core.Operation.

busunkim96 · 2021-05-19T16:53:08Z

samples/snippets/quickstart.py

+# [START compute_instances_operation_check]
+import typing
+
+import google.cloud.compute_v1 as gce


Style suggestion for consistency with existing python samples.

Suggested change

import google.cloud.compute_v1 as gce

from google.cloud import compute_v1

samples/snippets/quickstart.py

samples/snippets/test_quickstart.py

samples/snippets/quickstart.py

Co-authored-by: Bu Sun Kim <8822365+busunkim96@users.noreply.github.com>

samples/snippets/quickstart.py

m-strzelczyk · 2021-05-21T10:08:30Z

I think there's something wrong with Kokoro -samples tests. They're trying to use samples/snippets/noxfile.py which doesn't exist. The tests can be run from the root directory with nox -s samples.

m-strzelczyk · 2021-05-21T12:11:41Z

@busunkim96 I read the configs in .kokoro/samples and I think I could create a noxfile.py to match what they expect to find. However, I think it's a nicer solution to have one noxfile.py script in the root directory to handle all the testing.

Please let me know how should I proceed. Should I create noxfile.py in the snippets directory or modify the scripts in .kokoro/samples to match what I did now with the main noxfile.py.

This reverts commit 6d0b427

m-strzelczyk · 2021-05-25T05:42:46Z

@busunkim96 I have adjusted my tests to work with the automated kokoro scripts :)

m-strzelczyk · 2021-05-27T12:53:42Z

@dinagraves Can you review this PR?

busunkim96

The LRO handling still looks a little odd to me - I'll poke at the existing tests to see if there's a nicer option.

Otherwise this looks very nice!

busunkim96 · 2021-05-27T20:07:15Z

samples/snippets/quickstart.py

+
+    print(f"Creating the {instance_name} instance in {zone}...")
+    operation = instance_client.insert(request=request)
+    # wait_result = operation_client.wait(operation=operation.name, zone=zone, project=project)


Is this a stray comment?

Yes. Changed that part already.

busunkim96 · 2021-05-27T20:45:49Z

samples/snippets/quickstart.py

+    if operation.error:
+        pass
+    if operation.warnings:
+        pass
+    print(f"Instance {machine_name} deleted.")


Can the sample show error handling? (Even if it is just printing out the error/warnings?).

dinagraves

Generally LGTM. A few suggestions.

dinagraves · 2021-06-02T22:19:13Z

samples/snippets/README.md

+1. If you haven't already, set up a Python Development Environment by following the [python setup guide](https://cloud.google.com/python/setup) and 
+[create a project](https://cloud.google.com/resource-manager/docs/creating-managing-projects#creating_a_project).
+
+1. Create a service account with the 'Editor' permissions by following these 


I would recommend using gcloud auth application-default login instead. It's more secure if you don't have to download a JSON key. It's also easier. You can skip the next 3 steps with this method.

OK. Good point. Done.

dinagraves · 2021-06-02T22:20:58Z

samples/snippets/quickstart.py

+            "or set GOOGLE_APPLICATION_CREDENTIALS to use this script."
+        )
+    else:
+        test_instance_name = "i" + uuid.uuid4().hex[:10]


Why is this called test_instance_name?

Other note, can you concatenate something like "quickstart" to the instance name, so it's easier to identify? I ran this on a current project and got a little confused and worried before I recognized which one was the quickstart.

Changed to just instance_name and made the name start with quickstart-

* README suggests using `gcloud auth application-default login` which is safer than Service Account key. * The name of created instance now starts with "quickstart-". * Changed one variable name.

…amples

🤖 I have created a release \*beep\* \*boop\* --- ## [0.4.0](https://www.github.com/googleapis/python-compute/compare/v0.3.0...v0.4.0) (2021-06-07) ### Features * Adding code samples and tests for them ([#55](https://www.github.com/googleapis/python-compute/issues/55)) ([14cd352](https://www.github.com/googleapis/python-compute/commit/14cd352079281ddde3602597334e3c88c942ed30)) * Raise GoogleAPICallError on REST response errors ([01db23b](https://www.github.com/googleapis/python-compute/commit/01db23bc0af3ab30bfb5ad8205446ae49417f0f1)) * Re-generated to pick up changes from googleapis-discovery with latest fixes ([#60](https://www.github.com/googleapis/python-compute/issues/60)) ([01db23b](https://www.github.com/googleapis/python-compute/commit/01db23bc0af3ab30bfb5ad8205446ae49417f0f1)) * remove support for google-api-core < 1.28.0 ([01db23b](https://www.github.com/googleapis/python-compute/commit/01db23bc0af3ab30bfb5ad8205446ae49417f0f1)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

feat: Adding code samples and tests for them

edbabb2

m-strzelczyk requested review from a team as code owners May 19, 2021 14:24

m-strzelczyk requested a review from dinagraves May 19, 2021 14:24

product-auto-label bot added the api: compute Issues related to the googleapis/python-compute API. label May 19, 2021

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 19, 2021

product-auto-label bot added the samples Issues that are directly related to samples. label May 19, 2021

chore: Remove unused import.

5c53f10

m-strzelczyk mentioned this pull request May 19, 2021

Google cloud-client compute samples GoogleCloudPlatform/python-docs-samples#5841

Closed

8 tasks

busunkim96 reviewed May 19, 2021

View reviewed changes

rsamborski reviewed May 20, 2021

View reviewed changes

samples/snippets/quickstart.py Outdated Show resolved Hide resolved

m-strzelczyk and others added 2 commits May 20, 2021 13:23

Update .github/CODEOWNERS

9bca18c

Co-authored-by: Bu Sun Kim <8822365+busunkim96@users.noreply.github.com>

chore: Updating samples to meet reviewers suggestions.

1178ae0

rsamborski reviewed May 20, 2021

View reviewed changes

samples/snippets/quickstart.py Show resolved Hide resolved

chore: Fixing regions.

48b5162

m-strzelczyk added 4 commits May 24, 2021 13:00

chore: Adjusting the testing scripts in .kokoro/samples.

6d0b427

Revert "chore: Adjusting the testing scripts in .kokoro/samples."

3348ef4

This reverts commit 6d0b427

chore: Moving samples tests to dedicated noxfile.py.

1a36afa

chore: Adding 3.6 and 3.7 Python versions to samples noxfile.py

f2a02f1

m-strzelczyk added 2 commits May 25, 2021 07:48

chore: Updating the Samples section to reflect new testing setup.

0107c58

chore: Updating the Samples README.

8f0e869

m-strzelczyk requested a review from busunkim96 May 25, 2021 10:56

busunkim96 added 2 commits May 27, 2021 20:05

chore: add standardized samples noxfile

8040588

chore: uncomment sections in synth.py

4c9b0fb

busunkim96 reviewed May 27, 2021

View reviewed changes

m-strzelczyk added 2 commits June 1, 2021 14:08

chore: Changing the waiting for operation part.

572244a

Merge remote-tracking branch 'origin/samples' into samples

5e7d5c2

dinagraves approved these changes Jun 2, 2021

View reviewed changes

dinagraves and others added 3 commits June 2, 2021 15:25

Merge branch 'master' into samples

75a9d36

chore: Minor changes based on review.

c1670b9

* README suggests using `gcloud auth application-default login` which is safer than Service Account key. * The name of created instance now starts with "quickstart-". * Changed one variable name.

Merge branch 'samples' of github.com:googleapis/python-compute into s…

2bf4583

…amples

m-strzelczyk merged commit 14cd352 into master Jun 6, 2021

m-strzelczyk deleted the samples branch June 6, 2021 13:07

m-strzelczyk mentioned this pull request Jul 20, 2021

chore: update templated files #67

Merged

1 task

release-please bot mentioned this pull request Jan 13, 2022

chore(main): release 1.0.0 #191

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Adding code samples and tests for them #55

feat: Adding code samples and tests for them #55

m-strzelczyk commented May 19, 2021

snippet-bot bot commented May 19, 2021 •

edited

busunkim96 left a comment

busunkim96 May 19, 2021

rsamborski May 20, 2021

m-strzelczyk May 20, 2021

busunkim96 May 19, 2021

m-strzelczyk commented May 21, 2021

m-strzelczyk commented May 21, 2021

m-strzelczyk commented May 25, 2021

m-strzelczyk commented May 27, 2021

busunkim96 left a comment

busunkim96 May 27, 2021

m-strzelczyk Jun 1, 2021

busunkim96 May 27, 2021

m-strzelczyk Jun 1, 2021

dinagraves left a comment

dinagraves Jun 2, 2021

m-strzelczyk Jun 3, 2021

dinagraves Jun 2, 2021

dinagraves Jun 2, 2021

m-strzelczyk Jun 3, 2021

	import google.cloud.compute_v1 as gce
	from google.cloud import compute_v1

feat: Adding code samples and tests for them #55

feat: Adding code samples and tests for them #55

Conversation

m-strzelczyk commented May 19, 2021

snippet-bot bot commented May 19, 2021 • edited

busunkim96 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-strzelczyk commented May 21, 2021

m-strzelczyk commented May 21, 2021

m-strzelczyk commented May 25, 2021

m-strzelczyk commented May 27, 2021

busunkim96 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dinagraves left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snippet-bot bot commented May 19, 2021 •

edited