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

Server reflection calls should return transitive dependencies #1718

Closed
jroper opened this issue Nov 17, 2022 · 1 comment · Fixed by #1719
Closed

Server reflection calls should return transitive dependencies #1718

jroper opened this issue Nov 17, 2022 · 1 comment · Fixed by #1719

Comments

@jroper
Copy link
Contributor

jroper commented Nov 17, 2022

When the server reflection support returns file descriptors, it should not just return the file descriptor requested, but also all that file descriptors transitive dependencies. See:

    // This message is used to answer file_by_filename, file_containing_symbol,
    // file_containing_extension requests with transitive dependencies. As
    // the repeated label is not allowed in oneof fields, we use a
    // FileDescriptorResponse message to encapsulate the repeated fields.
    // The reflection service is allowed to avoid sending FileDescriptorProtos
    // that were previously sent in response to earlier requests in the stream.
    FileDescriptorResponse file_descriptor_response = 4;

Right now, we're only returning the file descriptor requested. This causes problems for clients that expect transitive dependencies to be included, and also makes server reflection very slow, because it means each descriptor must be fetched individually. We could of course implement support for tracking which we've already sent on a given stream, and not send them, that would be pretty straight forward.

@jroper
Copy link
Contributor Author

jroper commented Nov 17, 2022

Fixed in #1719.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants