Skip to content

Endpoints GRPC Python Sample #852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 20, 2017
Merged

Endpoints GRPC Python Sample #852

merged 7 commits into from
Mar 20, 2017

Conversation

ryanmats
Copy link
Contributor

@ryanmats ryanmats commented Mar 14, 2017

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.

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

@jeffmendoza jeffmendoza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.


# Copy Server
COPY . /hello/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


RUN pip install -r requirements.txt

EXPOSE 8000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove these options for java and objc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

ENV VIRTUAL_ENV -p python3.5 /env
ENV PATH /env/bin:$PATH

COPY requirements.txt /tmp/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use /app instead of /tmp.

The base image actually sets workdir to /app anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

from __future__ import print_function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't actually needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

single quotes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment here:

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

while True:
time.sleep(_ONE_DAY_IN_SECONDS)
except KeyboardInterrupt:
server.stop(0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the 0 here mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use stop(grace=0).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@theacodes theacodes dismissed jeffmendoza’s stale review March 20, 2017 21:48

issues addressed

@theacodes
Copy link
Contributor

@ryanmats nox should pass lint now. :)

@theacodes
Copy link
Contributor

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

@theacodes theacodes merged commit e6a5ff4 into master Mar 20, 2017
@theacodes theacodes deleted the endpoints-grpc branch March 20, 2017 22:23
Linchin pushed a commit that referenced this pull request Aug 18, 2025
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants