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

fix noenc error by fixing Details error field #557

Merged
merged 1 commit into from
Feb 24, 2018

Conversation

srenatus
Copy link
Contributor

As advised by @itizir in the discussion of #551.

However, I've gone with the minimal change to make the error go away, I
haven't introduced a proper, generated protobuf definition.

Also, I've noticed our error body message there looks suspiciously like
Status, except for the field name "Error" vs "Message".

Fixes #551.

As advised by @itizir in the discussion of grpc-ecosystem#551.

However, I've gone with the minimal change to make the error go away, I
haven't introduced a proper, generated protobuf definition.

Also, I've noticed our error body message there looks suspiciously like
Status[1], except for the field name "Error" vs "Message".

Fixes grpc-ecosystem#551.

[1]: https://github.com/google/go-genproto/blob/2b5a72b8730b0b/googleapis/rpc/status/status.pb.go#L83-L93

Signed-off-by: Stephan Renatus <[email protected]>
@achew22
Copy link
Collaborator

achew22 commented Feb 24, 2018

However, I've gone with the minimal change to make the error go away, I
haven't introduced a proper, generated protobuf definition.

In that PR we decided to not include the complete proto. Turns out it is comically large and dramatically increases the complexity of vendoring making it kind of a logistical nightmare.

Thanks for sending in the PR!

@achew22 achew22 merged commit d96e013 into grpc-ecosystem:master Feb 24, 2018
@srenatus
Copy link
Contributor Author

@achew22 thanks for sharing your rationale :)

@srenatus srenatus deleted the sr/fix-no-oenc-error branch February 25, 2018 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants