provide new endpoints regarding new service types - #5367
Conversation
| } | ||
|
|
||
| // ListAll returns array of all services | ||
| func (manager *Manager) ListAll() []*Instance { |
There was a problem hiding this comment.
There are many reasons to use non pointer slices. Less memory, fewer allocations needed and it's also faster. Pointer slices also serve little practical use, especially in this case here. It's almost always more trouble than its worth.
I see it's been done here like that before, am I just missing something?
There was a problem hiding this comment.
Actually yes, there's similar method called List, which also uses pointer instead of struct. So, I decided to go this way
| func (se *ServiceEndpoint) ServiceList(c *gin.Context) { | ||
| instances := se.serviceManager.List() | ||
| includeAll := false | ||
| includeAllStr := c.Request.URL.Query().Get("include_all") |
There was a problem hiding this comment.
There is c.Get or c.GetBool wouldn't those do the same but better?
There was a problem hiding this comment.
as I understand those methods doesn't fetch data from query params. it only looks inside c.Keys map[string]interface{}
| } | ||
|
|
||
| instances := make([]*service.Instance, 0) | ||
| if includeAll { |
There was a problem hiding this comment.
Could these be a method on the serviceManager itself? Would be nice to not have this if / else logic in the handler and the manager itself could decide if it should return all or some portion based on given params.
There was a problem hiding this comment.
good point, I thought about it too
…deAll to method List()
Codecov Report
@@ Coverage Diff @@
## master #5367 +/- ##
==========================================
- Coverage 38.66% 38.53% -0.14%
==========================================
Files 357 357
Lines 19476 19533 +57
==========================================
- Hits 7531 7527 -4
- Misses 11211 11267 +56
- Partials 734 739 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
5 participants
Update endpoint /services: add optional query param "include_all" (boolean). when include_all=true return all services, otherwise only running.
Add endpoint /v2/prices/current, which will return current prices for all service types.
According to #5262