Fix the readme gRPC usage example #122
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- fix a syntax error - fix a usage error
- this lib targets >= 3.6
boukeversteegh
previously approved these changes
Jul 20, 2020
abn
requested changes
Jul 20, 2020
README.md
Outdated
Comment on lines
172
to
174
| loop = asyncio.get_event_loop() | ||
| loop.run_until_complete(main()) | ||
|
|
There was a problem hiding this comment.
Suggested change
| loop = asyncio.get_event_loop() | |
| loop.run_until_complete(main()) | |
| if __name__ == '__main__': | |
| asyncio.run(main()) | |
There was a problem hiding this comment.
i specifically didn't use asyncio.run (cause of second commit) as It is introduced into the standard library as of python 3.7, since this library targets >= 3.6 this method cannot be used without breaking in that older interpreter.
Moving the runtime into a entry point check, however, is a good idea.
Optimized imports, store RPC call result before printing Co-authored-by: Arun Babu Neelicattu <arun.neelicattu@gmail.com>
boukeversteegh
approved these changes
Jul 21, 2020
Gobot1234
added a commit
to Gobot1234/python-betterproto
that referenced
this pull request
Jul 21, 2020
Gobot1234
added a commit
to Gobot1234/python-betterproto
that referenced
this pull request
Jul 27, 2020
Gobot1234
added a commit
to Gobot1234/python-betterproto
that referenced
this pull request
Aug 6, 2020
This reverts commit 2745953
Gobot1234
pushed a commit
to Gobot1234/python-betterproto
that referenced
this pull request
Aug 24, 2020
* re-implement README gRPC client example to be a self-contained script - fix a syntax error - fix a usage error * asyncio.run() was added in 3.7 - this lib targets >= 3.6 * Apply suggestions from code review Optimized imports, store RPC call result before printing Co-authored-by: Arun Babu Neelicattu <arun.neelicattu@gmail.com> * add entry-point check to example Co-authored-by: Arun Babu Neelicattu <arun.neelicattu@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The sample gRPC client implementation in the readme had two defects in it which this PR corrects.
The first is a simple syntax error in the
async forline, and should require no further explanation.The second is a library usage error relating to the application of
grpclib.client.Channel.The issue stems from
grpclib.client.Channelneeding to be manually closed at the end of the session, which the readme example failed to do. This leads to runtime warnings when the channel object gets GC'ed.As discussed in slack, not calling
channel.close()is a implementation error and is the primary motivation of this PR.Changes in tone:
I rewrote the client example to run from a script file instead of assuming a REPL, which I believe to be more realistic a use case.
The other examples in the README are, imho, sufficient to demonstrate the intended functionality.