Endpoints GRPC Python Sample by ryanmats · Pull Request #852 · GoogleCloudPlatform/python-docs-samples

ryanmats · 2017-03-14T23:56:13Z

Note: There are linting issues withe helloworld_pb2.py and helloworld_pb2_grpc.py but we plan on ignoring these issues since they are generated code.

…n-docs-samples into endpoints-grpc

jeffmendoza

Looks good overall.

jeffmendoza · 2017-03-15T00:14:48Z

endpoints/getting-started-grpc/Dockerfile

+
+# Copy Server
+COPY . /hello/
+


In general, dont use ADD. Should look something like:

COPY requirements.txt /tmp/ RUN pip install --requirement /tmp/requirements.txt COPY . /tmp/

see: https://docs.docker.com/engine/userguide/eng-image/dockerfile_best-practices/#add-or-copy

jeffmendoza · 2017-03-15T00:15:34Z

endpoints/getting-started-grpc/Dockerfile

+
+RUN pip install -r requirements.txt
+
+EXPOSE 8000


I believe this container is exposing 50051, but listing it is not necessary

jeffmendoza · 2017-03-15T00:17:01Z

endpoints/getting-started-grpc/helloworld/README.md

@@ -0,0 +1 @@
+[This code's documentation lives on the grpc.io site.](http://www.grpc.io/docs/quickstart/python.html)


Why keep the code under helloworld/? Doesn't make sense with only one sample. Either way this README is not needed.

removed the helloworld/ subdirectory and moved files to getting-started-grpc/ directory

jeffmendoza · 2017-03-15T00:18:36Z

endpoints/getting-started-grpc/protos/BUILD

@@ -0,0 +1,37 @@
+# Copyright 2017, Google Inc.


I don't think Python devs use bazel too much. Makes sense to remove this.

jeffmendoza · 2017-03-15T00:18:55Z

endpoints/getting-started-grpc/protos/helloworld.proto

+option java_package = "io.grpc.examples.helloworld";
+option java_outer_classname = "HelloWorldProto";
+option objc_class_prefix = "HLW";
+


You can remove these options for java and objc

theacodes · 2017-03-15T20:10:12Z

endpoints/getting-started-grpc/Dockerfile

+ENV VIRTUAL_ENV -p python3.5 /env
+ENV PATH /env/bin:$PATH
+
+COPY requirements.txt /tmp/


use /app instead of /tmp.

The base image actually sets workdir to /app anyway.

theacodes · 2017-03-15T20:11:32Z

endpoints/getting-started-grpc/README.md

+1. Generate the `out.pb` from the proto file:
+
+    ```bash
+    python -m grpc_tools.protoc --include_imports --include_source_info -I protos protos/helloworld.proto --descriptor_set_out out.pb


@jeffmendoza is out.pb the same filename we're using in the other examples? It seems like a bad name.

@jonparrott out.pb is what is used in the Java canonical sample (https://github.com/GoogleCloudPlatform/java-docs-samples/tree/master/endpoints/getting-started-grpc0). do you wanna change it to something else?

It's fine, I was just questioning Jeff, but he's on leave now so it's whatever. :)

theacodes · 2017-03-15T20:13:10Z

endpoints/getting-started-grpc/greeter_client.py

+
+"""The Python implementation of the GRPC helloworld.Greeter client."""
+
+from __future__ import print_function


This isn't actually needed

theacodes · 2017-03-15T20:13:18Z

endpoints/getting-started-grpc/greeter_client.py

+        metadata.append(('x-api-key', api_key))
+    response = stub.SayHello(
+        helloworld_pb2.HelloRequest(name='you'), metadata=metadata)
+    print("Greeter client received: " + response.message)


single quotes.

theacodes · 2017-03-15T20:14:17Z

endpoints/getting-started-grpc/greeter_server.py

+    helloworld_pb2_grpc.add_GreeterServicer_to_server(Greeter(), server)
+    server.add_insecure_port('[::]:50051')
+    server.start()
+    try:


Add a comment here:

# gRPC starts a new thread to service requests. Just make the main thread sleep.

theacodes · 2017-03-15T20:14:37Z

endpoints/getting-started-grpc/greeter_server.py

+        while True:
+            time.sleep(_ONE_DAY_IN_SECONDS)
+    except KeyboardInterrupt:
+        server.stop(0)


What's the 0 here mean?

http://www.grpc.io/grpc/python/grpc.html#grpc.Server.stop

"grace – A duration of time in seconds or None. If a duration of time in seconds, the time to allow existing RPCs to complete before being aborted by this Server’s stopping. If None, all RPCs will be aborted immediately and this method will block until this Server is completely stopped."

Let me know what you want to do with this

use stop(grace=0).

…n-docs-samples into endpoints-grpc

issues addressed

theacodes · 2017-03-20T21:49:12Z

@ryanmats nox should pass lint now. :)

theacodes · 2017-03-20T21:49:38Z

@dpebot will you merge when travis is green? pretty please?

* chore(deps): update all dependencies * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

ryanmats added 2 commits March 14, 2017 16:54

Endpoints GRPC Python sample

6c76b63

Merge branch 'master' of https://github.com/GoogleCloudPlatform/pytho…

af5f9a4

…n-docs-samples into endpoints-grpc

ryanmats requested review from jeffmendoza and theacodes March 14, 2017 23:56

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 14, 2017

jeffmendoza previously requested changes Mar 15, 2017

View reviewed changes

Made changes based on Jeff's first code review

b758290

theacodes suggested changes Mar 15, 2017

View reviewed changes

ryanmats and others added 4 commits March 15, 2017 17:51

Made changes based on Jon's code review

3915587

Merge branch 'master' of https://github.com/GoogleCloudPlatform/pytho…

23c6d58

…n-docs-samples into endpoints-grpc

fixed grace period issue

e7ad7cc

Make nox ignore pb2 and pb2_grpc files

729c008

theacodes approved these changes Mar 20, 2017

View reviewed changes

theacodes merged commit e6a5ff4 into master Mar 20, 2017

theacodes deleted the endpoints-grpc branch March 20, 2017 22:23

Linchin mentioned this pull request Aug 18, 2025

chore: migrate handwritten samples from googleapis/python-bigquery-storage #13543

Merged

9 tasks

		@@ -0,0 +1 @@
		[This code's documentation lives on the grpc.io site.](http://www.grpc.io/docs/quickstart/python.html)


		"""The Python implementation of the GRPC helloworld.Greeter client."""

		from __future__ import print_function

Conversation

ryanmats commented Mar 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeffmendoza left a comment

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!

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!

theacodes commented Mar 20, 2017

Uh oh!

theacodes commented Mar 20, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ryanmats commented Mar 14, 2017 •

edited

Loading