Conversation
| return nil, errors.E(err) | ||
| } | ||
|
|
||
| // Shouldn't happen, the Juju client presumes that |
There was a problem hiding this comment.
If we assume it won't happen, we should probably at least log if it does. Worth considering panicking as well.
There was a problem hiding this comment.
Yeah happy to add, the juju client code doesn't care about this but I wanted to just add a little precaution.
There was a problem hiding this comment.
The other option is to remove the check entirely and let the index below panic. Up to you.
| // UpdateCloudsCredentialForce updates the given credential on the controller. | ||
| // It calls UpdateCloudsCredentials, and adapts it to: | ||
| // 1. Take a single credential | ||
| // 2. Always force the update (force=true). |
There was a problem hiding this comment.
So AFAICT, our previous code does the check + update separately to enable batch checking across controllers, as we update the credential for all controllers currently holding it.
SimoneDutto
left a comment
There was a problem hiding this comment.
i'm not fully clued up on this piece of work, but looks good to me
Before merging, I'll explain tomorrow in standup so it is clearer |
we update to the tip to include the new checkcredentialmodels client method
updates cloud method to return the cloud and not mutate pointer, this makes the client package consistent and enables us to remove it at a later date
refactors check credentialmodels and updatecredential to call the error parsing logic in a consistent location. additionally renames update credentials to reflect what it actually does.
ec95e8b to
bf6c65d
Compare
Description
This PR updates the cloud client to use the juju client.
The primary changes of this PR are:
There's a little remaining work to refactor
updateControllerCloudCredential, UpdateCloudsCredentialForce and CheckCredentialsModels actually take different parameters and we need this in our client right now. It would be preferable we don't have this in our client (again, in the event we wish to just delete the file).Engineering checklist