Changed AllocationEndpoint to array of endpoints #830
Conversation
|
Build Failed 😱 Build Id: 27067c47-d1aa-4aa9-b092-b2c5dca83032 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if response.StatusCode >= 500 && (i+1) < len(connectionInfo.AllocationEndpoints) { |
There was a problem hiding this comment.
| if response.StatusCode >= 500 && (i+1) < len(connectionInfo.AllocationEndpoints) { | |
| if response.StatusCode >= 500 { |
I think that this check is redundant for loop will not continue if we are out of range connectionInfo.AllocationEndpoints.
There was a problem hiding this comment.
Thanks. This condition says if the there is any other endpoint try sending the request with the new endpoint. Otherwise return the error. If I remove the condition, no error will be returned
|
Thanks for the pull request. You should run you could also write |
|
Build Succeeded 👏 Build Id: 0b4ffcfd-dd70-4864-aaac-41d0fd598d9f 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:
|
| // If there is a server error try a different endpoint | ||
| continue | ||
| } | ||
| if response.StatusCode >= 400 { |
There was a problem hiding this comment.
Curious - why stop at a 400, why not continue to the next endpoint like a 500?
There was a problem hiding this comment.
4xx means there is an error from client. We only retry with new endpoint if there is an error from server e.g. 5xx.
There was a problem hiding this comment.
My concern would be, that if after I set everything up, and all is well - if something weird happens to one of my clusters, such that they return a 404, that could potentially stop all/a large chunk of multicluster allocations.
There was a problem hiding this comment.
404 means the client is asking gameserverallocation for a resource that is not available. It is a client error and should be handled by client. If a cluster is out of gameservers to allocate, then it returns 500. Does it make sense?
There was a problem hiding this comment.
Oh yeah totally 👍 I'm just thinking through worst case scenarios.
Like someone accidentally puts a different service in place of where the allocator used to be setup 😨
Not incredibly likely, but just thinking through it.
markmandel
added
area/user-experience
|
Build Failed 😱 Build Id: 5f9fffce-ded5-4e94-a9d8-5c5a506181b0 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
| // If there is a server error try a different endpoint | ||
| continue | ||
| } | ||
| if response.StatusCode >= 400 { |
There was a problem hiding this comment.
My concern would be, that if after I set everything up, and all is well - if something weird happens to one of my clusters, such that they return a 404, that could potentially stop all/a large chunk of multicluster allocations.
|
Build Failed 😱 Build Id: 64423a31-2aa7-4fa8-9c11-82b00cfd3229 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Build Succeeded 👏 Build Id: 297945a0-f14a-4588-bf5d-6c1cd566e03d 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:
|
No description provided.