Implementing cross cluster allocation request #757
Conversation
pooneh-m
commented
May 3, 2019
pooneh-m
commented
- Get secret that stores certificates.
- Call remote allocation server.
|
Build Failed 😱 Build Id: b289b99d-b508-4daf-a6b4-007c3e9692be To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Succeeded 👏 Build Id: f43386c7-7774-4a03-8260-847a81b45739 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:
|
| return nil, errors.New(string(data)) | ||
| } | ||
|
|
||
| err = json.Unmarshal(data, &gsaResult) |
There was a problem hiding this comment.
nit:
this can help avoid having to explicitly read data:
if err := json.NewUnmarshaler(response.Body).Decode(&gsaResult); err != nil {
return nil, fmt.Errors("unable to parse request: %v", err)
}
There was a problem hiding this comment.
The problem is this would fail for error cases that does not decode to gsa. So it actually needs the two step deserialization.
There was a problem hiding this comment.
not sure what's going on here, then
is it important to return data in error string? if so, can you add some comments?
There was a problem hiding this comment.
The data is returned as an error string only for http status 4xx and 5xx. In that case the response body is expected to contain error and not GSA message. Therefore Decode(&gsaResult) would fail to parse the response body as GSA.
I'll add comment in the next PR since this is already merged.
|
Build Succeeded 👏 Build Id: bf2d0b36-6a19-4e51-b9dc-19f98c8c9da3 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:
|
|
Build Succeeded 👏 Build Id: f2245c0f-ad4e-455b-8eb5-325802d9bebc 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
area/user-experience
