Skip to content

Add python async example for hellostreamingworld using generator#27343

Merged
lidizheng merged 4 commits intogrpc:masterfrom
mickours:add-async-hellostreamingworld-python-example
Jan 6, 2022
Merged

Add python async example for hellostreamingworld using generator#27343
lidizheng merged 4 commits intogrpc:masterfrom
mickours:add-async-hellostreamingworld-python-example

Conversation

@mickours
Copy link
Copy Markdown
Contributor

Following of the issue #24603 .

The problem was a missing example not a bug in the gRPC implementation, so here is an example of async Unary Stream call using the already present hellostreamingworld.proto definition.

It uses a message async generator out of the object to provide an example for that kind of usage.

@yashykt

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Sep 14, 2021

CLA Signed

The committers are authorized under a signed CLA.

@clayg
Copy link
Copy Markdown
Contributor

clayg commented Sep 21, 2021

@lidizheng this example looks great to me! definately something I would have liked to have when I was exploring the examples for the first time.

@clayg
Copy link
Copy Markdown
Contributor

clayg commented Sep 21, 2021

I guess we already have a more complicated async streaming example:

https://github.com/grpc/grpc/tree/master/examples/python/async_streaming

and all the other helloworld async tests are in a different directory:

https://github.com/grpc/grpc/tree/master/examples/python/helloworld

Copy link
Copy Markdown
Contributor

@lidizheng lidizheng left a comment

Choose a reason for hiding this comment

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

Thanks! This example looks great. Just have some style comments.

Comment thread examples/python/hellostreamingworld/async_greeter_client.py Outdated
Comment thread examples/python/hellostreamingworld/async_greeter_server.py Outdated
self, request: HelloRequest,
context: grpc.aio.ServicerContext) -> HelloReply:
logging.info(f"Serving sayHello request {request}")
async for msg in say_hello_generator(request):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would recommend to simplify port the message generation here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've kept it that way to show how you can integrate with external generator (this was my first use case for this example)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see what you are proposing. This example file focuses on how to write a streaming gRPC server handler. Using an extra layer of generator doesn't seem to yield more helpful info for readers.

Comment thread examples/python/hellostreamingworld/async_greeter_server.py Outdated
Comment thread examples/python/hellostreamingworld/async_greeter_client.py Outdated
Copy link
Copy Markdown
Contributor

@lidizheng lidizheng left a comment

Choose a reason for hiding this comment

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

Looks great. Can you check the result of the Sanity Tests?

self, request: HelloRequest,
context: grpc.aio.ServicerContext) -> HelloReply:
logging.info(f"Serving sayHello request {request}")
async for msg in say_hello_generator(request):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see what you are proposing. This example file focuses on how to write a streaming gRPC server handler. Using an extra layer of generator doesn't seem to yield more helpful info for readers.

@stale
Copy link
Copy Markdown

stale Bot commented Jan 3, 2022

This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 30 days. It will be closed automatically if no further update occurs in 7 day. Thank you for your contributions!

@mickours
Copy link
Copy Markdown
Contributor Author

mickours commented Jan 4, 2022

Just waiting for a review. Anyone available?

@lidizheng
Copy link
Copy Markdown
Contributor

@mickours The sanity checks can be fixed by running:

tools/distrib/yapf_code.sh
tools/distrib/isort_code.sh

You can also check for pylint errors locally: tools/distrib/pylint_code.sh

@mickours
Copy link
Copy Markdown
Contributor Author

mickours commented Jan 6, 2022

I've apply the auto-format and lint scripts. Should be OK now.

@lidizheng
Copy link
Copy Markdown
Contributor

Known failure: #28483

@lidizheng lidizheng merged commit b784a43 into grpc:master Jan 6, 2022
@mickours mickours deleted the add-async-hellostreamingworld-python-example branch January 7, 2022 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lang/Python release notes: yes Indicates if PR needs to be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants