Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions connectrpc-grpcreflect/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ The services are standard Connect services that you use as normal.
from typing import cast

from connectrpc_grpcreflect import (
ServerReflectionAlphaASGIApplication,
ServerReflectionAlphaService,
ServerReflectionASGIApplication,
ServerReflectionService,
)
Expand All @@ -26,11 +28,22 @@ eliza_app = ElizaServiceASGIApplication(ElizaService())
reflection_app = ServerReflectionASGIApplication(
ServerReflectionService(eliza_pb.desc())
)
reflection_alpha_app = ServerReflectionAlphaASGIApplication(
ServerReflectionAlphaService(eliza_pb.desc())
)

app = Starlette(
routes=[
Mount(eliza_app.path, cast(ASGIApp, eliza_app)),
Mount(reflection_app.path, cast(ASGIApp, reflection_app)),
Mount(reflection_alpha_app.path, cast(ASGIApp, reflection_alpha_app)),
]
)
```

Mount both `grpc.reflection.v1` and `grpc.reflection.v1alpha`. Both are

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

listed by `list_services`, and a client that calls v1alpha gets a 404 if it
isn't mounted.

Reflection uses the gRPC protocol which requires a server that supports HTTP/2
trailers such as [pyvoy](https://pyvoy.dev).
193 changes: 91 additions & 102 deletions connectrpc-grpcreflect/connectrpc_grpcreflect/_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,15 @@

from connectrpc.code import Code
from connectrpc.errors import ConnectError
from protobuf import DescEnum, DescExtension, DescFile, DescService, Oneof, Registry
from protobuf import (
DescEnum,
DescExtension,
DescFile,
DescMessage,
DescService,
Oneof,
Registry,
)

from ._gen.grpc.reflection.v1.reflection_connect import (
ServerReflection,
Expand Down Expand Up @@ -57,9 +65,7 @@ def __init__(self, *descs: DescFile | DescService) -> None:
*descs: The descriptors to make available for reflection.

"""
registry, service_names = _resolve_registry(descs)
self._registry = registry
self._service_names = service_names
self._index = _Index(descs)

async def server_reflection_info(
self,
Expand All @@ -68,7 +74,7 @@ async def server_reflection_info(
) -> AsyncIterator[ServerReflectionResponse]:
seen: set[str] = set()
async for req in request:
yield _handle_request(req, seen, self._registry, self._service_names)
yield _handle_request(req, seen, self._index)


class ServerReflectionServiceSync(ServerReflectionSync):
Expand All @@ -90,9 +96,7 @@ def __init__(self, *descs: DescFile | DescService) -> None:
*descs: The descriptors to make available for reflection.

"""
registry, service_names = _resolve_registry(descs)
self._registry = registry
self._service_names = service_names
self._index = _Index(descs)

def server_reflection_info(
self,
Expand All @@ -101,7 +105,7 @@ def server_reflection_info(
) -> Iterator[ServerReflectionResponse]:
seen: set[str] = set()
for req in request:
yield _handle_request(req, seen, self._registry, self._service_names)
yield _handle_request(req, seen, self._index)


# v1 and v1alpha are binary compatible, to keep the implementation simple but still
Expand All @@ -127,9 +131,7 @@ def __init__(self, *descs: DescFile | DescService) -> None:
*descs: The descriptors to make available for reflection.

"""
registry, service_names = _resolve_registry(descs)
self._registry = registry
self._service_names = service_names
self._index = _Index(descs)

async def server_reflection_info(
self,
Expand All @@ -144,8 +146,7 @@ async def server_reflection_info(
_handle_request(
ServerReflectionRequest.from_binary(req.to_binary()),
seen,
self._registry,
self._service_names,
self._index,
).to_binary()
)

Expand All @@ -169,9 +170,7 @@ def __init__(self, *descs: DescFile | DescService) -> None:
*descs: The descriptors to make available for reflection.

"""
registry, service_names = _resolve_registry(descs)
self._registry = registry
self._service_names = service_names
self._index = _Index(descs)

def server_reflection_info(
self,
Expand All @@ -186,69 +185,95 @@ def server_reflection_info(
_handle_request(
ServerReflectionRequest.from_binary(req.to_binary()),
seen,
self._registry,
self._service_names,
self._index,
).to_binary()
)


def _resolve_registry(
descs: tuple[DescFile | DescService, ...],
) -> tuple[Registry, list[str]]:
fds: list[DescFile] = []
seen: set[str] = set()
service_names: set[str] = set()
for desc in (*descs, ServerReflection.desc(), ServerReflectionAlpha.desc()):
if isinstance(desc, DescFile):
fds.extend(_file_descriptor_with_dependencies(desc, seen))
service_names.update(d.type_name for d in desc.services)
else:
fds.extend(_file_descriptor_with_dependencies(desc.file, seen))
service_names.add(desc.type_name)
return Registry(*fds), sorted(service_names)
class _Index:
"""The files available for reflection, and lookups into them."""

def __init__(self, descs: tuple[DescFile | DescService, ...]) -> None:
files: list[DescFile] = []
seen: set[str] = set()
service_names: set[str] = set()
for desc in (*descs, ServerReflection.desc(), ServerReflectionAlpha.desc()):
if isinstance(desc, DescFile):
files.extend(_file_descriptor_with_dependencies(desc, seen))
service_names.update(d.type_name for d in desc.services)
else:
files.extend(_file_descriptor_with_dependencies(desc.file, seen))
service_names.add(desc.type_name)
self.registry = Registry(*files)
self.service_names = sorted(service_names)
# Every symbol that C++ protobuf's
# DescriptorPool::FindFileContainingSymbol resolves, mapped to the file
# declaring it.
self.symbols: dict[str, DescFile] = {}
# Fully qualified names of messages, mapped to their extension numbers.
self.extension_numbers: dict[str, list[int]] = {}
for desc in self.registry:
if isinstance(desc, DescFile):
continue
self.symbols[desc.type_name] = desc.file
match desc:
case DescMessage():
# The descriptor's oneof_decl includes synthetic oneofs for
# proto3 optional fields, which desc.oneofs omits.
for name in (
*(f.name for f in desc.fields),
*(o.name for o in desc.proto.oneof_decl),
):
self.symbols[f"{desc.type_name}.{name}"] = desc.file
case DescEnum():
# Enum values are siblings of their enum, not children.
scope = desc.type_name.removesuffix(desc.name)
for value in desc.values:
self.symbols[scope + value.name] = desc.file
case DescService():
for method in desc.methods:
self.symbols[f"{desc.type_name}.{method.name}"] = desc.file
case DescExtension():
self.extension_numbers.setdefault(
desc.extendee.type_name, []
).append(desc.number)


def _handle_request(
req: ServerReflectionRequest,
seen: set[str],
registry: Registry,
service_names: list[str],
req: ServerReflectionRequest, seen: set[str], index: _Index
) -> ServerReflectionResponse:
res = ServerReflectionResponse(valid_host=req.host, original_request=req)
match req.message_request:
case Oneof("file_by_filename", filename):
desc = registry.file(filename)
res.message_response = _create_file_response(desc, seen)
res.message_response = _create_file_response(
index.registry.file(filename), seen, "file not found"
)
case Oneof("file_containing_symbol", symbol):
desc = _find_file_for_symbol(registry, symbol)
res.message_response = _create_file_response(desc, seen)
res.message_response = _create_file_response(
index.symbols.get(symbol), seen, "symbol not found"
)
case Oneof("file_containing_extension", extension_request):
desc: DescFile | None = None
ext = registry.extension_for(
ext = index.registry.extension_for(
extension_request.containing_type, extension_request.extension_number
)
if ext:
desc = ext.file
res.message_response = _create_file_response(desc, seen)
res.message_response = _create_file_response(
ext.file if ext else None, seen, "extension not found"
)
case Oneof("all_extension_numbers_of_type", containing_type):
# We should return not found if the message itself isn't registered
if not registry.message(containing_type):
res.message_response = _create_file_response(None, seen)
if not index.registry.message(containing_type):
res.message_response = _not_found("type not found")
else:
nums: list[int] = [
d.number
for d in registry
if isinstance(d, DescExtension)
and d.extendee.type_name == containing_type
]
res.message_response = Oneof(
"all_extension_numbers_response",
ExtensionNumberResponse(
base_type_name=containing_type, extension_number=nums
base_type_name=containing_type,
extension_number=index.extension_numbers.get(
containing_type, []
),
),
)
case Oneof("list_services", _):
services = [ServiceResponse(name=name) for name in service_names]
services = [ServiceResponse(name=name) for name in index.service_names]
res.message_response = Oneof(
"list_services_response", ListServiceResponse(service=services)
)
Expand All @@ -257,17 +282,21 @@ def _handle_request(
return res


def _not_found(message: str) -> Oneof[Literal["error_response"], ErrorResponse]:
return Oneof(
"error_response",
ErrorResponse(error_code=_CODE_NOT_FOUND, error_message=message),
)


def _create_file_response(
desc: DescFile | None, seen: set[str]
desc: DescFile | None, seen: set[str], not_found_message: str
) -> (
Oneof[Literal["file_descriptor_response"], FileDescriptorResponse]
| Oneof[Literal["error_response"], ErrorResponse]
):
if not desc:
return Oneof(
"error_response",
ErrorResponse(error_code=_CODE_NOT_FOUND, error_message="symbol not found"),
)
return _not_found(not_found_message)
fds = [
fd.proto.to_binary() for fd in _file_descriptor_with_dependencies(desc, seen)
]
Expand All @@ -291,43 +320,3 @@ def _file_descriptor_with_dependencies(
seen.add(file.name)
yield file
queue.extend(file.dependencies)


# Finds the file declaring a fully-qualified symbol name: any message, enum,
# service, extension, method, field, oneof, or enum value, matching
# DescriptorPool::FindFileContainingSymbol in C++ protobuf.
def _find_file_for_symbol(registry: Registry, symbol: str) -> DescFile | None:
desc = (
registry.message(symbol)
or registry.enum(symbol)
or registry.service(symbol)
or registry.extension(symbol)
)
if desc:
return desc.file
# May be a fully qualified method, field, oneof, or enum value, split off the
# parent and find it.
parent, _, member = symbol.rpartition(".")
if not member:
return None
if svc := registry.service(parent):
if any(m.name == member for m in svc.methods):
return svc.file
return None
if (msg := registry.message(parent)) and (
any(f.name == member for f in msg.fields)
# The descriptor's oneof_decl includes synthetic oneofs for proto3
# optional fields, which msg.oneofs omits.
or any(o.name == member for o in msg.proto.oneof_decl)
):
return msg.file
# An enum value of a message-nested enum also has a message as its parent
# scope, so always finish with the enum value scan.
for d in registry:
if (
isinstance(d, DescEnum)
and d.type_name.rpartition(".")[0] == parent
and any(v.name == member for v in d.values)
):
return d.file
return None
19 changes: 17 additions & 2 deletions connectrpc-grpcreflect/test/test_grpcreflect.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,15 +293,30 @@ async def test_not_found(reflection_client: ReflectionClient) -> None:
"file_containing_symbol", "connectrpc.example.Hat.missing"
)
),
ServerReflectionRequest(
message_request=Oneof(
"file_containing_extension",
ExtensionRequest(
containing_type="connectrpc.example.Hat", extension_number=1
),
)
),
],
)

assert len(responses) == 4
errors = []
for res in responses:
assert res.message_response is not None
assert res.message_response.field == "error_response"
assert res.message_response.value.error_code == 5
assert res.message_response.value.error_message
errors.append(res.message_response.value.error_message)
assert errors == [
"file not found",
"symbol not found",
"type not found",
"symbol not found",
"extension not found",
]


@pytest.mark.asyncio
Expand Down
Loading