Skip to content
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

Add Python Reflection Client #28443

Merged
merged 5 commits into from
Feb 14, 2022
Merged

Conversation

tomerv
Copy link
Contributor

@tomerv tomerv commented Dec 29, 2021

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

@donnadionne

@linux-foundation-easycla
Copy link

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

linux-foundation-easycla bot commented Dec 29, 2021

CLA Signed

The committers are authorized under a signed CLA.

@tomerv
Copy link
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
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.

@tomerv
Copy link
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
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.

Mostly improve documentation.
Copy link
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
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
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
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

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