Make ensure-availability a bulk API call. #115
Conversation
| return params.StateServersChanges{}, fmt.Errorf("invalid environment tag: %v", err) | ||
| } | ||
| _, err := c.api.state.FindEntity(spec.EnvironTag) | ||
| if err != nil { |
There was a problem hiding this comment.
Please use an inline err check like is done in the ParseEnvironTag call above
|
Looks pretty good. We are missing some testing coverage though for the case where an invalid environ tag is passed in. |
|
I am not so savvy on this part of juju to pass a more meaningful review, but my 2c here, ill agree with @wallyworld I see a not so trivial set of changes and you did not touch a single test file, I would have expected some tests broken after this, if not it is a great opportunity to increase coverage. |
|
wallyworld, perito666 - thanks for the reviews, I've fixed the style issues and added a test for a call with an invalid environment tag |
| @@ -772,14 +772,22 @@ func (c *Client) APIHostPorts() ([][]network.HostPort, error) { | |||
|
|
|||
| // EnsureAvailability ensures the availability of Juju state servers. | |||
| func (c *Client) EnsureAvailability(numStateServers int, cons constraints.Value, series string) (params.StateServersChanges, error) { | |||
| var result params.StateServersChanges | |||
| var results params.StateServersChangeResults | |||
| args := params.StateServersSpec{ | |||
There was a problem hiding this comment.
I'd just do:
args := params.StateServersSpecs{
Specs: []params.StateServersSpec{{
EnvironTag: c.st.EnvironTag(),
NumStateServers: numStateServers,
Constraints: cons,
Series: series,
}}}
|
A few comments about making the API changes more consistent, but otherwise looks good. |
|
|
||
| // StateServersChangeResult contains the results | ||
| // of a single EnsureAvailability API call, or | ||
| // the error.. |
|
Thank you! LGTM with tiny a doc comment change. |
|
|
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
Make ensure-availability a bulk API call. Makin ensure-availability a bulk API call.
minor fixes found by go vet
Build the about hosted-olm page
Makin ensure-availability a bulk API call.