Add missing GCF samples (other than tutorials) by ace-n · Pull Request #1614 · GoogleCloudPlatform/python-docs-samples

ace-n · 2018-08-07T01:09:11Z

No description provided.

ace-n · 2018-08-11T02:14:49Z

@andrewsg something broke in Nox.py, and it no longer supports linting a specific directory. (You have to lint all of python-docs-samples.)

Can you (or another Python person) submit a PR to address this?

ace-n · 2018-08-13T17:42:08Z

Edit: addressed the Nox issues in #1630. Force pushed to this branch to include the new changes.

Change-Id: I2362e4ee8031fd3ea6880ffcec5eb253615df5ba

Change-Id: Ieb2b5b3421bd6821bc6d5626b7986c23e9d82350

Change-Id: I0fb311114bf8c3554e2f052ca00f1dd9eef1cf69

Change-Id: I5ed6d6dc3792e28b12162c47c7e6f5c974a7477f

Change-Id: I1a17e1c72bea6e7a8d18b9ed5910db09bedb753d

Change-Id: I3d99531c68b5ca3272cb6208777f3ab9bde8f9bd

Change-Id: I06c2a5cb46fd2cb497af7ba9e02e43a4e97de972

Change-Id: Ic20ff72602ea3b6b1fdc361d87aea7dfb742150f

Change-Id: Ie8682514d99aa6f82786a76719535efd58a5c1bf

Change-Id: Ief7e96a43948af12de32627b20031c95953ade09

Change-Id: Idc9854ba256271081078c15f7e62d3fd6bbec449

Change-Id: Ib1b6f8eb47511fe9b19ed6d773b4f9c6ef0dc8ed

andrewsg

Why are all of the requirements files extended?

ace-n · 2018-08-13T22:59:36Z

IIRC believe this step was required to get the builds passing previously.

That being said, I'll revert it and see if the build still passes now that the Nox issues have been fixed.

andrewsg · 2018-08-13T23:33:09Z

It looks like that removed requirements.txt entirely for some samples such as slack and tips.

Change-Id: I8fe4f70869deeb3aac0f9c6016f5c5e45a172731

ace-n · 2018-08-14T00:01:15Z

Removed that commit in favor of shrinking the existing requirements.txt files. PTAL once the build passes?

Change-Id: Iba2165bd2af99306feb47b7e29640ebf7d8f3e95

ace-n · 2018-08-14T19:09:01Z

Build is passing - PTAL?

Change-Id: I585f2a6a31cf5e23a54eb02fb366a8685601ea23

andrewsg · 2018-08-14T20:03:14Z

functions/helloworld/requirements.txt

@@ -0,0 +1 @@
+pytest==3.6.0


Please remove this one too.

andrewsg · 2018-08-14T20:03:44Z

functions/log/main.py

+# [END functions_log_retrieve]
+
+# [START functions_logs_retrieve]
+from google.cloud import logging as cloud_logging


Why "import as" here instead of just "import google.cloud.logging"?

andrewsg · 2018-08-14T20:04:23Z

functions/log/main.py

+# [START functions_log_retrieve]
+cloud_client = cloud_logging.Client()
+log_name = 'cloudfunctions.googleapis.com%2Fcloud-functions'
+cloud_logger = cloud_client.logger(log_name.format(os.getenv('GCP_PROJECT')))


Do we mention in the readme or anywhere else that this env variable has to be set for it to work locally?

Not currently - added.

andrewsg · 2018-08-14T20:05:20Z

functions/tips/main_test.py

-from unittest.mock import MagicMock, Mock, patch

 import flask
+from mock import MagicMock, Mock, patch


Why did this need to be changed?

andrewsg · 2018-08-14T20:05:32Z

functions/tips/main_test.py

 import flask
+from mock import MagicMock, Mock, patch
 import pytest
+


Was this change required by the linter?

Nope - removed.

andrewsg · 2018-08-14T20:05:58Z

functions/tips/requirements.txt

 google-cloud-error-reporting==0.30.0
-python-dateutil==2.7.3
+google-cloud-pubsub==0.35.4
+mock==2.0.0


This is not needed as it's included in testing requirements.

Change-Id: Ibb5a518f60d06c7637cc39ab54e81a234ea188ec

andrewsg · 2018-08-15T18:41:31Z

functions/http/main.py

+# [END functions_http_signed_url]
+
+# [START functions_http_form_data]
+from werkzeug.utils import secure_filename


I believe this will work as-is, even without werkzeug in requirements.txt, but have you tested it in production just to make sure (since clearing requirements.txt)?

Made some (unrelated) bugfixes and confirmed it works.

Change-Id: If71dba3d86954e8e0bf7168638c5c73eb40fe599

ace-n · 2018-08-15T23:48:44Z

@dpebot merge when green

dpebot · 2018-08-15T23:48:44Z

Okay! I'll merge when all statuses are green and all reviewers approve.

ace-n requested a review from andrewsg August 7, 2018 01:09

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 7, 2018

Ace Nassri added 11 commits August 13, 2018 10:49

Add GCP API call sample

34384f6

Change-Id: I2362e4ee8031fd3ea6880ffcec5eb253615df5ba

Add log retrieval sample + a few import tweaks

22a6792

Change-Id: Ieb2b5b3421bd6821bc6d5626b7986c23e9d82350

Move log retrieval sample

8dda20d

Change-Id: I0fb311114bf8c3554e2f052ca00f1dd9eef1cf69

Add XML parsing sample

b3c24c0

Change-Id: I5ed6d6dc3792e28b12162c47c7e6f5c974a7477f

Add multipart sample

b901336

Change-Id: I1a17e1c72bea6e7a8d18b9ed5910db09bedb753d

Add docstrings

28b83e0

Change-Id: I3d99531c68b5ca3272cb6208777f3ab9bde8f9bd

Add signed URL sample

fcfbd96

Change-Id: I06c2a5cb46fd2cb497af7ba9e02e43a4e97de972

Fix nits

6497b05

Change-Id: Ic20ff72602ea3b6b1fdc361d87aea7dfb742150f

Add requirements.txt files

d24f879

Change-Id: Ie8682514d99aa6f82786a76719535efd58a5c1bf

Build fix: downgrade 'futures'

07e062a

Change-Id: Ief7e96a43948af12de32627b20031c95953ade09

Fix failing test + s/Exception/runtimeError/

247eecf

Change-Id: Idc9854ba256271081078c15f7e62d3fd6bbec449

ace-n force-pushed the gcf-low-pri branch from 524f856 to 182d167 Compare August 13, 2018 17:53

Remove incorrect lint commits + actually fix lint

bd4cac8

Change-Id: Ib1b6f8eb47511fe9b19ed6d773b4f9c6ef0dc8ed

ace-n force-pushed the gcf-low-pri branch from 182d167 to bd4cac8 Compare August 13, 2018 18:10

andrewsg requested changes Aug 13, 2018

View reviewed changes

Shrink requirements.txt files

7d82dd4

Change-Id: I8fe4f70869deeb3aac0f9c6016f5c5e45a172731

ace-n force-pushed the gcf-low-pri branch from d1a3403 to 7d82dd4 Compare August 14, 2018 00:00

Add missing package

0fd4798

Change-Id: Iba2165bd2af99306feb47b7e29640ebf7d8f3e95

Remove add'l pkgs

6395ee5

Change-Id: I585f2a6a31cf5e23a54eb02fb366a8685601ea23

andrewsg requested changes Aug 14, 2018

View reviewed changes

Address Andrew's comments

c457ab5

Change-Id: Ibb5a518f60d06c7637cc39ab54e81a234ea188ec

andrewsg approved these changes Aug 15, 2018

View reviewed changes

Fix minor python{2->3} bugs

a14c6d8

Change-Id: If71dba3d86954e8e0bf7168638c5c73eb40fe599

ace-n force-pushed the gcf-low-pri branch from 8baf558 to a14c6d8 Compare August 15, 2018 23:47

dpebot added the automerge Merge the pull request once unit tests and other checks pass. label Aug 15, 2018

dpebot self-assigned this Aug 15, 2018

ace-n merged commit 69543d4 into master Aug 16, 2018

ace-n deleted the gcf-low-pri branch August 16, 2018 01:21

		@@ -0,0 +1 @@
		pytest==3.6.0

Conversation

ace-n commented Aug 7, 2018

Uh oh!

ace-n commented Aug 11, 2018

Uh oh!

ace-n commented Aug 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewsg left a comment

Choose a reason for hiding this comment

Uh oh!

ace-n commented Aug 13, 2018

Uh oh!

andrewsg commented Aug 13, 2018

Uh oh!

ace-n commented Aug 14, 2018

Uh oh!

ace-n commented Aug 14, 2018

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!

Choose a reason for hiding this comment

Uh oh!

ace-n Aug 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ace-n commented Aug 15, 2018

Uh oh!

dpebot commented Aug 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ace-n commented Aug 13, 2018 •

edited

Loading

ace-n Aug 15, 2018 •

edited

Loading