Skip to content

Add Python Reflection Client#28443

Merged
lidizheng merged 5 commits intogrpc:masterfrom
tomerv:python-reflection-client
Feb 14, 2022
Merged

Add Python Reflection Client#28443
lidizheng merged 5 commits intogrpc:masterfrom
tomerv:python-reflection-client

Conversation

@tomerv
Copy link
Copy Markdown
Contributor

@tomerv tomerv commented Dec 29, 2021

Implement ProtoReflectionDescriptorDatabase in Python to support
client-side reflection sevices.

@donnadionne

@linux-foundation-easycla
Copy link
Copy Markdown

CLA Not Signed

Implement ProtoReflectionDescriptorDatabase in Python to support
client-side reflection sevices.
@tomerv tomerv force-pushed the python-reflection-client branch from 9f7618f to f6c85d1 Compare December 29, 2021 10:49
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Dec 29, 2021

CLA Signed

The committers are authorized under a signed CLA.

@tomerv
Copy link
Copy Markdown
Contributor Author

tomerv commented Jan 16, 2022

How can I make progress with this?
@gnossen @lidizheng

@lidizheng lidizheng added kokoro:run lang/Python release notes: yes Indicates if PR needs to be in release notes labels Jan 18, 2022
@lidizheng
Copy link
Copy Markdown
Contributor

Wow, this is an interesting proposal. A programmatic way to access descriptors from client-side. This PR alters gRPC Python's surface API, which requires a gRFC proposal see https://github.com/grpc/proposal. It doesn't need to be long or very detail about implementation. You can extend from the "Use Server Reflection in a Python client" section.

Comment thread doc/server_reflection_tutorial.md Outdated
Comment thread src/python/grpcio_reflection/grpc_reflection/v1alpha/BUILD.bazel Outdated
Comment thread src/python/grpcio_tests/tests/reflection/_reflection_servicer_test.py Outdated
@tomerv
Copy link
Copy Markdown
Contributor Author

tomerv commented Jan 20, 2022

I opened a PR for a proposal, and I will handle all the comments in this PR soon.

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 for the update. Looks pretty good now.

If you have any question, please let me know.

Comment thread src/python/grpcio_reflection/grpc_reflection/v1alpha/BUILD.bazel Outdated
Comment thread src/python/grpcio_tests/tests/reflection/BUILD.bazel Outdated
Comment thread src/python/grpcio_tests/tests/reflection/_reflection_client_test.py
Comment thread src/python/grpcio_tests/tests/reflection/_reflection_client_test.py Outdated
Comment thread src/python/grpcio_tests/tests/reflection/_reflection_servicer_test.py Outdated
Comment thread doc/python/server_reflection.md
Comment thread src/python/grpcio_tests/tests/reflection/_reflection_servicer_test.py Outdated
Mostly improve documentation.
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.

@gnossen PTAL.

LGTM. Thanks for adding the documents! It looks great.


Our test seems is suffering external breakage. We might need to merge master after it's fixed.

@lidizheng
Copy link
Copy Markdown
Contributor

According to the presubmit checks, you may want to add the new test case to https://github.com/grpc/grpc/blob/master/src/python/grpcio_tests/tests/tests.json.

@tomerv
Copy link
Copy Markdown
Contributor Author

tomerv commented Feb 10, 2022

According to the presubmit checks, you may want to add the new test case to https://github.com/grpc/grpc/blob/master/src/python/grpcio_tests/tests/tests.json.

Done. Sorry for the delay.
I couldn't get the test to run correctly on my machine, so I hope my change is correct.

@lidizheng
Copy link
Copy Markdown
Contributor

@tomerv Can you take a look at the Sanity Checks result? https://source.cloud.google.com/results/invocations/66d0ab27-16cc-4972-b54c-bc55e5d2cc4f/targets

Most of the scripts can be run directly to format code for you.

@linux-foundation-easycla
Copy link
Copy Markdown

CLA Not Signed

@tomerv tomerv force-pushed the python-reflection-client branch from 190f15f to 81fb2f8 Compare February 13, 2022 07:40
@tomerv
Copy link
Copy Markdown
Contributor Author

tomerv commented Feb 14, 2022

Seems like the checks are stuck. @lidizheng, can you help?

@lidizheng lidizheng merged commit 3e8e229 into grpc:master Feb 14, 2022
drfloob added a commit that referenced this pull request Feb 14, 2022
nicolasnoble pushed a commit that referenced this pull request Feb 15, 2022
lidizheng added a commit that referenced this pull request Feb 15, 2022
@copybara-service copybara-service Bot added the imported Specifies if the PR has been imported to the internal repository label Feb 15, 2022
lidizheng added a commit that referenced this pull request Mar 2, 2022
drfloob added a commit that referenced this pull request Mar 4, 2022
drfloob added a commit that referenced this pull request Mar 4, 2022
@lidizheng lidizheng added release notes: no Indicates if PR should not be in release notes and removed release notes: yes Indicates if PR needs to be in release notes labels Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

imported Specifies if the PR has been imported to the internal repository lang/Python release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants