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

feat: use google.rpc.Status #411

Merged
1 commit merged into from
Nov 17, 2022
Merged

Conversation

yuryu
Copy link
Member

@yuryu yuryu commented Nov 17, 2022

What type of PR is this?
/kind feat

What this PR does / Why we need it:
Use the standard google.rpc.Status in GetRecordsResponse instead of the custom defined proto for better interoperability with the gRPC Go library.
This is a backward-compatible change at the proto level.

Which issue(s) this PR fixes:

Closes #

Special notes for your reviewer:

Use the standard google.rpc.Status in GetRecordsResponse
instead of the custom defined proto.
This is a backward-compatible change at the proto level.
@yuryu yuryu requested review from a user and hongalex November 17, 2022 03:19
Record: protoRec,
StoreKey: storeKey,
Status: pbStatus,
Record: rec.ToProto(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice simplification of the logic with the use of the Status from grpc and changes in the record.Record functions (to test for nil).

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -0,0 +1,3 @@
[submodule "third_party/googleapis"]
path = third_party/googleapis
url = https://github.com/googleapis/googleapis.git
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to see that importing the proto file this way works! In my previous attempts, I copied the file from googleapis locally in api/google/grpc/status.proto but had issues with the server code that was serializing the response at runtime.

@ghost ghost merged commit 6f66aad into googleforgames:main Nov 17, 2022
@yuryu yuryu deleted the google-rpc-status branch November 17, 2022 05:56
This pull request was closed.
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 this pull request may close these issues.

1 participant