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

Use separate response messages in our gRPC API #8563

Open
emilk opened this issue Dec 26, 2024 · 2 comments
Open

Use separate response messages in our gRPC API #8563

emilk opened this issue Dec 26, 2024 · 2 comments
Labels
remote-store remote store gRPC API

Comments

@emilk
Copy link
Member

emilk commented Dec 26, 2024

It is best practice to use a different response message to each rpc call, as that allows us to add more fields to the response in the future without sacrificing compatibility.

That is, instead of

    rpc QueryCatalog(QueryCatalogRequest) returns (stream DataframePart) {}
    rpc RegisterRecording(RegisterRecordingRequest) returns (DataframePart) {}

we should have

    rpc QueryCatalog(QueryCatalogRequest) returns (stream QueryCatalogResponse) {}
    rpc RegisterRecording(RegisterRecordingRequest) returns (RegisterRecordingResponse) {}
@emilk emilk added the remote-store remote store gRPC API label Dec 26, 2024
@grtlr
Copy link
Contributor

grtlr commented Jan 3, 2025

I can do this as part of #8566.

@zehiko @jleibs Right now, the gRPC API uses a lot of dataframes instead of dedicated types. I think we should start to define more dedicated types (and optionally add a request parameter to return a data frame if still required).

@zehiko
Copy link
Contributor

zehiko commented Jan 7, 2025

the most recent API changes were made as part of https://github.com/rerun-io/dataplatform/issues/74 (which is good to read for understanding the motivation behind the changes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
remote-store remote store gRPC API
Projects
None yet
Development

No branches or pull requests

3 participants