grpc icon indicating copy to clipboard operation
grpc copied to clipboard
verified publisher icon grpc

[reflection]: python: reflection returns `original_request`

Open Drarig29 opened this issue 1 year ago • 7 comments

Closes #36943

This PR adds original_request to any _reflection_pb2.ServerReflectionResponse generated by grpc_reflection.v1alpha

Drarig29 avatar Jun 17 '24 13:06 Drarig29

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: Drarig29 / name: Corentin Girard (e6a94789e1445e7f8c1adeb6509e96cbb1e08ee6, ebc1f0eafb277a5ff23889ac4d3a45e928abece7, ff151529e626211cbdae29815bde3f4ece78dce8)

linux-foundation-easycla[bot] avatar Jun 17 '24 13:06 linux-foundation-easycla[bot]

cc. @gnossen @XuanWang-Amos (as assignees to the related issue)

Drarig29 avatar Jun 17 '24 13:06 Drarig29

@Drarig29 Thanks for the contribution . Please Add unit-tests for the suggested changes.

sourabhsinghs avatar Jun 18 '24 16:06 sourabhsinghs

You're welcome! - added the unit tests

Drarig29 avatar Jun 18 '24 20:06 Drarig29

Hi @sreenithi! I updated the unit tests and they passed locally. FYI, the "Mergeable" check is failing because of labels that I cannot change.

Drarig29 avatar Jun 19 '24 10:06 Drarig29

Hi @sreenithi! Gentle bump on the PR, is anything blocking?

Drarig29 avatar Jun 28 '24 15:06 Drarig29

Hi, apologies for the delay. The code looks good to me. I will review and try to merge it this coming week.

sreenithi avatar Jun 28 '24 17:06 sreenithi

Hi @sreenithi and @sourabhsinghs! Is it expected that this PR wasn't released yet?

Could it be due to the failing CI on the merge commit? We have a Python service that requires that change in order to work properly (like other gRPC servers).

Drarig29 avatar Sep 03 '24 15:09 Drarig29

hi @sreenithi and @sourabhsinghs we are having the same problem in one of our services, when we can expect this fix to be released?

alvico avatar Sep 04 '24 13:09 alvico

Hi @drfloob!

I'm pinging you as you are a member of the grpc org and you published a release recently. Could you have a look?

Thanks a lot! 🙇

Drarig29 avatar Sep 04 '24 13:09 Drarig29

Hi @Drarig29 , this PR has been merged to the master branch last month (see it here), there were no issues because of the failing CI test, but required some internal changes, and hence there was a delay initially, however, it is already merged at the moment. Let me check and revert on which release will include this change.

sreenithi avatar Sep 05 '24 06:09 sreenithi

@sreenithi did you find out which release will include this change?

alvico avatar Sep 13 '24 09:09 alvico

Hi, yes, it will be included in the upcoming release 1.67.0 which is due for release in the next 1-2 weeks

sreenithi avatar Sep 13 '24 19:09 sreenithi

Hi, the 1.67.0 was released on PyPi today. You should now be able to use this fix

sreenithi avatar Oct 16 '24 08:10 sreenithi