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

Unable to add protobuf wrappers in url template option #808

Closed
mayank-dixit opened this issue Nov 14, 2018 · 10 comments · Fixed by #816
Closed

Unable to add protobuf wrappers in url template option #808

mayank-dixit opened this issue Nov 14, 2018 · 10 comments · Fixed by #816

Comments

@mayank-dixit
Copy link

mayank-dixit commented Nov 14, 2018

Pre

  1. Using string value from: https://github.com/protocolbuffers/protobuf/blob/master/src/google/protobuf/wrappers.proto
  2. protoc version: 3.6.1

Steps

My proto file looks like this:

helloworld.proto

syntax = "proto3";

package helloworld;

import "google/api/annotations.proto";
import "google/protobuf/wrappers.proto";

service Greeter {
  rpc SayHello (HelloRequest) returns (HelloReply) {
    option (google.api.http) = {
      get: "/say/{strVal}"
    };
  }
}

message HelloRequest {
  google.protobuf.StringValue strVal = 5;
}

message HelloReply {
  string message = 1;
}

And then I try compile protos:

protoc -I/usr/local/include -I. \
  -I$GOPATH/src \
  -I$GOPATH/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
  --go_out=plugins=grpc:. \
  helloworld/helloworld.proto

Works fine. And then I try to generate reverse proxy using:

protoc -I/usr/local/include -I. \
  -I$GOPATH/src \
  -I$GOPATH/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
  --grpc-gateway_out=logtostderr=true:. \
  helloworld/helloworld.proto

expected:

Successful compilation

result:

--grpc-gateway_out: aggregate type TYPE_MESSAGE in parameter of Greeter.SayHello: strVal
@johanbrandhorst
Copy link
Collaborator

I've no idea why this is erroring, have you had a chance to do some debugging?

@mayank-dixit
Copy link
Author

Yes, I tried to debug:
Error is thrown from:
https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-grpc-gateway/descriptor/services.go#L221
Where it doesn't find *target.TypeName(.google.protobuf.StringValue) to be a well known type.

Recognised well-known types are added here:
https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-grpc-gateway/descriptor/types.go#L453

@johanbrandhorst
Copy link
Collaborator

Huh, interesting, perhaps we just need to add the rest of the well known types there?

@mayank-dixit
Copy link
Author

I'm adding other types. It's passing. I'll verify end to end once. if all goes fine, I'll raise a PR?

@johanbrandhorst
Copy link
Collaborator

Yes, thanks a lot!

@achew22
Copy link
Collaborator

achew22 commented Nov 14, 2018

Fixed by #809. Thanks for your PR!

@johanbrandhorst
Copy link
Collaborator

It appears the MR discussed #809 just generates invalid code. @mayank-dixit are you interested in submitting a PR which adds the functions?

@johanbrandhorst
Copy link
Collaborator

Nevermind, still confused, continue discussion in #809

@johanbrandhorst
Copy link
Collaborator

Reopening as there's definitely a problem with the current code, see #809 (comment)

@johanbrandhorst
Copy link
Collaborator

Still not fixed, reopening

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