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

grpc-gateway: do no require manual grpc gateway registration #22715

Open
julienrbrt opened this issue Dec 2, 2024 · 5 comments · May be fixed by #22941
Open

grpc-gateway: do no require manual grpc gateway registration #22715

julienrbrt opened this issue Dec 2, 2024 · 5 comments · May be fixed by #22941
Assignees
Labels
C:server/v2 Issues related to server/v2

Comments

@julienrbrt
Copy link
Member

Follow-up of #22701, #20798

Our goal is to kill that explicit gRPC gateway registration (RegisterGRPCGatewayRoutes and generated code), and do some proto options parsing directly in the grpcgateway server. This will allow us to let user have modules even smaller and no SDK dependency and gRPC dependencies at all (roughly this: #20798 (comment))

ref: #22701 (review)

While gRPC gateway has been wired in #22701, as said in the message above, the goal is to removal totally the grpc gateway manual registration from the modules. This still needs to be implemented.

@julienrbrt julienrbrt added the C:server/v2 Issues related to server/v2 label Dec 2, 2024
@julienrbrt julienrbrt self-assigned this Dec 2, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Cosmos-SDK Dec 2, 2024
@tac0turtle tac0turtle moved this from 📋 Backlog to ☃️ Icebox in Cosmos-SDK Dec 2, 2024
@julienrbrt
Copy link
Member Author

In addition to modules, we should allow the registration of the following routes in the server (extensible like the gRPC server): https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.2/runtime/app.go#L224-L231

@technicallyty
Copy link
Contributor

I am interested in taking a stab at this!

@julienrbrt
Copy link
Member Author

I am interested in taking a stab at this!

Cool, thanks!

@technicallyty
Copy link
Contributor

technicallyty commented Dec 16, 2024

kill that explicit gRPC gateway registration (RegisterGRPCGatewayRoutes and generated code)

i'm not sure that this is possible with removing gateway code. the custom endpoint from google.api.http.get get generated into the gateway (*.pb.gw.go) code.

if we keep the gw generated code, im still not sure how to proceed as the custom endpoint is not exported. it's used internally in the mux registration code.

any idea on how to proceed here? is there an approach/element to this im missing?

@julienrbrt
Copy link
Member Author

julienrbrt commented Dec 16, 2024

The idea was mainly to ditch that whole generated code, but keep the google.api.http.get annotations.
This simplify the module.go of modules, and their dependencies.

The gRPC gateway server (server/v2/api/grpc-gateway), would have a catch all handler to answer to any path, and that handler would dynamically try to figure out which message was aimed the request.

For example:

  • GET /cosmos/bank/v1beta1/params -> split (3 first?) / -> cosmos.bank.v1beta1 -> Find service -> Find Params
    Then you can do a POST request to the internal rest endpoint (or just copy the logic in the grpc-gateway server) server/v2/api/rest)

That should work for most of them, this is when there are parameters that things get hacky:

  • GET /cosmos/bank/v1beta1/balances/cosmos123 -> split (3 first?) / -> cosmos.bank.v1beta1 -> Find service -> Loop over the whole service to find matching rpc call
    Then using the rest v2 stuff again

Again, that server won't be pretty, and probably slower than with the generated code, but it is more important to have killed the generated code + sdk import in modules. gRPC gateway server is there for backward compat (as handlers do not work with it). The idea is to have everyone slowly migrate to rest v2.

@julienrbrt julienrbrt moved this from ☃️ Icebox to 🤸‍♂️ In Progress in Cosmos-SDK Dec 16, 2024
@technicallyty technicallyty linked a pull request Dec 18, 2024 that will close this issue
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:server/v2 Issues related to server/v2
Projects
Status: 🤸‍♂️ In Progress
Development

Successfully merging a pull request may close this issue.

2 participants