Repository navigation
Index reflection symbols and fix README reflection mounts - #453
Conversation
bf7f3a9 to
61452c7
Compare
`_Index` builds maps from fully qualified symbol names to files and from message names to extension numbers once, at construction, instead of scanning the registry on each `file_containing_symbol` and `all_extension_numbers_of_type` request. This replaces `_find_file_for_symbol` and the registry setup repeated in each of the four services. Not-found errors now name what was missing (`file not found`, `extension not found`, `type not found`) instead of always saying `symbol not found`. Signed-off-by: Stefan VanBuren <stefan@vanburen.xyz>
`list_services` lists `grpc.reflection.v1alpha.ServerReflection` since #407, but the README example only mounted v1, so a client calling v1alpha got `unimplemented: HTTP status 404 Not Found`. The README also now says which servers reflection works on. Against Uvicorn (HTTP/1.1 only), `grpcurl list` fails with `context deadline exceeded` and `buf curl --list-services` with `the RPC protocol or method requires HTTP/2`, for every `--protocol`. Granian serves HTTP/2 but lacks the ASGI trailers extension, so `grpcurl` and `buf curl --protocol grpc` fail with `ASGI server does not support ASGI trailers extension`. pyvoy passes `grpcurl list`/`describe` and `buf curl --list-services` for every `--protocol` against both v1 and v1alpha. Signed-off-by: Stefan VanBuren <stefan@vanburen.xyz>
61452c7 to
de06443
Compare
| ) | ||
| ``` | ||
|
|
||
| Mount both `grpc.reflection.v1` and `grpc.reflection.v1alpha`. Both are |
There was a problem hiding this comment.
I didn't notice the list_services implication before. Should we go ahead and combine them into one app since the path dispatch should be simple?
There was a problem hiding this comment.
I think I'd leave that for a follow-up, if that works for you? one app can't really have a single path covering both (/grpc.reflection.v1.ServerReflection and /grpc.reflection.v1alpha.ServerReflection don't share a segment, so Mount(app.path, app) doesn't work), and I think connect-go keeps them as separate handlers too. we could add a helper that returns both routes though; happy to explore that separately.
There was a problem hiding this comment.
put up #460 for this; it adds reflection_apps (+ reflection_apps_sync) that returns both the v1 and v1alpha apps, since a single combined app still can't be mounted at one path.
| Reflection is a bidirectional streaming RPC, so the server needs HTTP/2, | ||
| and `grpcurl` speaks only the gRPC protocol, which needs the ASGI trailers | ||
| extension. Serve the app with [pyvoy](https://github.com/curioswitch/pyvoy), | ||
| which supports both; Uvicorn (HTTP/1.1 only) and Granian (no trailers) | ||
| don't. |
There was a problem hiding this comment.
| Reflection is a bidirectional streaming RPC, so the server needs HTTP/2, | |
| and `grpcurl` speaks only the gRPC protocol, which needs the ASGI trailers | |
| extension. Serve the app with [pyvoy](https://github.com/curioswitch/pyvoy), | |
| which supports both; Uvicorn (HTTP/1.1 only) and Granian (no trailers) | |
| don't. | |
| Reflection uses the gRPC protocol which requires a server that supports HTTP/2 trailers such as [pyvoy](https://pyvoy.dev). |
Co-authored-by: Anuraag (Rag) Agrawal <198344+anuraaga@users.noreply.github.com> Signed-off-by: Stefan VanBuren <stefan@vanburen.xyz>
Signed-off-by: Stefan VanBuren <stefan@vanburen.xyz>
This builds the symbol and extension-number lookups once up front instead of scanning the registry per request, and gives each not-found case its own message. The speedup is small in practice (about 10µs → 3µs for
all_extension_numbers_of_typeon a registry withdescriptor.protoand friends; file responses are dominated by serializing the descriptor anyway), so this is mostly about deleting_find_file_for_symbol.The README example only mounted v1, but we've listed v1alpha since #407, so a v1alpha client got a 404. Also noted which servers reflection works on: I checked
grpcurlandbuf curlagainst uvicorn (fails, no HTTP/2), granian (fails for gRPC since it lacks ASGI trailers), and pyvoy (works).