Change allocator gRPC response state to gRPC error status #1516
Conversation
|
Build Failed 😱 Build Id: bdcc5cce-43b3-479b-ba15-abef6dfa6ab5 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Failed 😱 Build Id: 9bca5d25-953f-4c09-91b4-6614fb575677 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Failed 😱 Build Id: 06073577-623a-4ac8-a3ff-6208eee7d2c3 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Succeeded 👏 Build Id: f52c9289-3b77-4210-afa8-0de4295715f0 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
|
|
||
| func validateAllocatorResponse(t *testing.T, resp *pb.AllocationResponse) { | ||
| t.Helper() | ||
| assert.Greater(t, len(resp.Ports), 0) |
There was a problem hiding this comment.
Could assert.Equal(..., 1) or other specific value/func parameter be here?
There was a problem hiding this comment.
Thanks. For this test >0 is enough for validation.
pooneh-m
changed the title
| @@ -183,13 +181,11 @@ func TestAllocatorCrossNamespace(t *testing.T) { | |||
| logrus.WithError(err).Info("failing Allocate request") | |||
There was a problem hiding this comment.
Line 173 above
logrus.WithError(err).Info("failing http request")
Should be "failing grpc.Dial" as in other places.
There was a problem hiding this comment.
No this is actually failing the allocate request. grpc.Dial is successful at this point.
There was a problem hiding this comment.
I mean not in this exact place but above. This is from master:
agones/test/e2e/allocator_test.go
Line 175 in 6a02b31
go.sum
Outdated
| @@ -290,6 +290,7 @@ google.golang.org/grpc v1.17.0/go.mod h1:6QZJwpn2B+Zp71q/5VxRsJ6NXXVCE5NRUHRo+f3 | |||
| google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= | |||
| google.golang.org/grpc v1.20.1 h1:Hz2g2wirWK7H0qIIhGIqRGTuMwTE8HEKFnDZZ7lm9NU= | |||
| google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= | |||
| google.golang.org/grpc v1.29.1 h1:EC2SB8S04d2r73uptxphDSUG+kTKVgjRPF+N3xpxRB4= | |||
There was a problem hiding this comment.
I am curious why go.mod was not changed?
It would still contain as in master?
google.golang.org/grpc v1.20.1
There was a problem hiding this comment.
Hmm. I am not sure. I just did go mod tidy and it went away.
|
Build Succeeded 👏 Build Id: 65144384-2549-48c1-b3ec-2af6f02d85e5 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aLekSer, pooneh-m The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Thanks for your thorough review. |
|
Build Succeeded 👏 Build Id: 020a35b8-d1c8-4af6-b55d-bd5409cee745 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
|
New changes are detected. LGTM label has been removed. |
|
Build Succeeded 👏 Build Id: 2d78ed6c-687a-4a15-8d71-0ab6294b2717 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
markmandel
added
kind/breaking
…ames#1516) Co-authored-by: Mark Mandel <markmandel@google.com>
What type of PR is this?
/kind breaking
What this PR does / Why we need it:
Allocator service gRPC API is changed to instead of returning response.status.state as allocated, unallocated or conflict return gRPC error status in case of failure and drop response.status.state API from the response.
Which issue(s) this PR fixes:
Closes #1040