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

Add default HTTP routes for non-annotated methods #123

Open
anhnmt opened this issue Apr 12, 2024 · 8 comments · May be fixed by #124
Open

Add default HTTP routes for non-annotated methods #123

anhnmt opened this issue Apr 12, 2024 · 8 comments · May be fixed by #124

Comments

@anhnmt
Copy link
Contributor

anhnmt commented Apr 12, 2024

I have the following program: https://github.com/anhnmt/gprc-dynamic-proto/blob/main/cmd/service/main.go

I have successfully connected grpc and restapi, can I call the api like the following?

curl --header "Content-Type: application/json" \
    --data '{}' \
    http://localhost:8080/user.v1.UserService/List

I tried and the result was: Not Found

@emcfarlane
Copy link
Collaborator

emcfarlane commented Apr 12, 2024

Hey @anhnmt, default bindings aren't provided yet but it's something we would be interested in adding. On registration of an endpoint like:

service UserService {
  rpc List(ListRequest) returns (ListResponse) {
    option (google.api.http) = {get: "/v1/users/{page=*}"};
  }
}

We should create an additional endpoint with the effective syntax:

    option (google.api.http) = {
      post: "/user.v1.UserService/List"
      body: "*"
    };

Edit: currently to call your API you would need to use the REST syntax:

curl http://localhost:8080/v1/users/1?page_size=10

@anhnmt
Copy link
Contributor Author

anhnmt commented Apr 12, 2024

@emcfarlane thank u

I tried your method and it worked, but I would have to fix a lot of other methods to get the same result. I wonder if there is any way to optimize this workload, maybe without having to interfere with the .proto file?

rpc List(ListRequest) returns (ListResponse) {
    option (google.api.http) = {
      get: "/v1/users/{page=*}"
      additional_bindings {
        post: "/user.v1.UserService/List"
        body: "*"
      }
    };
  }

@emcfarlane
Copy link
Collaborator

Yes, we plan to add bindings for the default service method similar to https://cloud.google.com/endpoints/docs/grpc/transcoding#where_to_configure_transcoding

@anhnmt
Copy link
Contributor Author

anhnmt commented Apr 13, 2024

@emcfarlane Hopefully this feature will be released soon

@anhnmt anhnmt closed this as completed Apr 13, 2024
@emcfarlane emcfarlane linked a pull request Apr 16, 2024 that will close this issue
@jhump
Copy link
Member

jhump commented Apr 16, 2024

@anhnmt, how are you actually using Vanguard? This default mapping doesn't seem to provide any value over just using the Connect protocol.

So with no annotations in the proto files you could do this:

curl --header "Content-Type: application/json" \
    --header "Connect-Protocol-Version: 1" \
    --data '{}' \
    http://localhost:8080/user.v1.UserService/List

The only difference between that, and explicitly configuring the service with the annotation (and not including the connect-protocol-version header) is the way that errors are represented. The error body will be JSON in both cases. With the annotations in place, the "RESTful" error format is the JSON format of the google.rpc.Status proto. With Connect, it will be slightly different (link to spec). In particular, Connect formats the code as a string instead of a number, so the response is more human-readable. Also, Connect uses a format for error details that is more robust. The "RESTful" format can fail to be rendered if the transcoding server doesn't have the Protobuf schemas for all detail messages, but the Connect format is not brittle in this way.

So do you actually need the "RESTful" format? Or would simply using the Connect protocol suffice?

@jhump jhump reopened this Apr 16, 2024
@anhnmt
Copy link
Contributor Author

anhnmt commented Apr 17, 2024

Hi @jhump, I'm developing an api gateway that uses vanguard to bridge to my back-end services.

My client will call this API like: http://localhost:8000/user.v1.UserService/List and it will automatically forward this request to my service: http://localhost:8080/user.v1.UserService/List.

In addition to the RESTful api, it also supports connecting to services using gPRC as well as the path on /user.v1.UserService/List.

Until recently, I wanted to upload files through this gateway and encountered the Not Found problem above. I tried your method in this case and it still doesn't work.

curl --location 'localhost:8080/user.v1.UserService/Upload' \
--header 'Connect-Protocol-Version: 1' \
--header 'Content-Type: application/json' \
--form 'file=@"/C:/Users/anhnmt/Downloads/build.zip"'

My current way is to use an option as follows

  rpc Upload(UploadRequest) returns (google.protobuf.Empty) {
    options (google.api.http) = {
      post: "/user.v1.UserService/Upload"
      body: "file"
    };
  }
  
  message UploadRequest {
  // The file contents to upload.
  google.api.HttpBody file = 1;
}

I wonder if you have a better way?

@jhump
Copy link
Member

jhump commented Apr 17, 2024

Since your annotation uses body: "file", it must be explicitly defined, vs. relying on some default route (which @emcfarlane mentioned adding). Also, The Connect protocol does not handle google.api.HttpBody as that is only a feature of the REST transcoding.

Prior to your latest comment, the example cURL commands and discussion appear to just be ad-hoc usage of some of the API endpoints that were unary JSON, in which case the Connect protocol would work just as well.

Your prior example showed having to add a route for /user.v1.UserService/List even though it already had an annotation for /v1/users/{page=*}. So why would the client be using the former? Why not use the URLs in the existing HTTP annotations?

@anhnmt
Copy link
Contributor Author

anhnmt commented Apr 17, 2024

@jhump Sorry for causing misunderstanding. We use the main route /user.v1.UserService/List and /v1/users/{page=*} for testing purposes only.

My model looks like: Client -> Gateway -> Services. The port will automatically connect to services using host information such as: host:port. The gateway will discover and connect them on its own, where /user.v1.UserService/List will be how we forward to the correct service.

However, my old system doesn't work well and is unstable. Recently I wanted to add some more features including file uploads to the system, we rebuilt it and tried to change as little as possible.

@emcfarlane emcfarlane changed the title How can I call the api? Add default HTTP routes for non-annotated methods Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants