tonic-reflection: add back support for v1alpha reflection protocol #1888
Conversation
djc
force-pushed
the
alpha-reflection
branch
3 times, most recently
from
9722b05
to
d557364
Compare
tonic-reflection 0.12.x moved from the v1alpha to v1 of the reflection protocol. However, most clients, like Postman, Kreya and evans don't support that one yet. Bump tonic-reflection to 0.12.2, which re-introduces v1alpha support alongside the v1 version of it, registering both services. This fixes the example documented in tvix/store/README.md, it was previously failing as evans couldn't find the v1alpha reflection service. See hyperium/tonic#1888 for details. Change-Id: I55438877317f82dc39face13afeb9594cda07a4e Reviewed-on: https://cl.tvl.fyi/c/depot/+/12353 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
|
From a client perspective how should this properly be handled, differentiating between when to use For instance: If I'm going to attempt to invoke a |
|
@BButner I don't have enough context on the nuances of gRPC versioning to answer this with any certainty. I suspect most clients will already know which version of the reflection their peer supports? If you (or anyone else) comes up with a good solution for this, kindly share it. |
|
@djc Yeah, that's a fair point. Specifically my use-case is with developing a tool to test gRPC servers, so being able to automatically tell which version they have is important. That does sound more like a me problem, and not a you/library problem. Right now my plan is:
|
|
Sounds about right to me. |
Motivation
In tonic 0.12, we upgraded to v1 reflection. However, some tools (like Postman and Kreya) still use the v1alpha1 version of the reflection schema. This PR tries to add back the v1alpha reflection schema with minimal code duplication, by moving code around so that the
Builderbuilds a schema-independentReflectionServiceStatewhich can be wrapped in a server following the requested schema version.This should supersede #1787 and #1822 (thanks @ttkjesper for the input -- I've stolen your tests) while reducing code duplication.
Would be happy to get feedback on whether this works for people affected by this issue, as I'd like to get this merged into the pending 0.12.2 release.