Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JUJU-593] Add APIVersionInfo to the controller #95

Merged
merged 1 commit into from
Feb 15, 2022

Conversation

hmlanigan
Copy link
Member

Allows for verification of the MAAS server and to get the version and subversion strings. Capabilities are cached when NewController is called.

@hmlanigan hmlanigan changed the title Add APIVersionInfo to the controller [JUJU-593] Add APIVersionInfo to the controller Feb 10, 2022
Copy link
Member

@manadart manadart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. One thing to consider.

@@ -94,7 +94,7 @@ func newControllerWithVersion(baseURL, apiVersion, apiKey string) (Controller, e
Minor: minor,
}
controller := &controller{client: client, apiVersion: controllerVersion}
controller.capabilities, err = controller.readAPIVersionInfo()
_, _, controller.capabilities, err = controller.readAPIVersionInfo()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we cache the capabilities here, there's less point in APIVersionInfo querying fresh and returning potentially (admittedly extremely unlikely) diverging info.

I'd be inclined to cache the version and subversion too, and just return the members in APIVersionInfo without hitting the API again.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally I'd agree, however the purpose here is to have a light weight api call specifically to check connectivity. Otherwise it'd be a call to return the api endpoint used by the client, and call Ping inside the provided.

@hmlanigan
Copy link
Member Author

$$merge$$

@jujubot jujubot merged commit f1f3892 into juju:v2 Feb 15, 2022
@hmlanigan hmlanigan deleted the controller-api-version-info branch February 15, 2022 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants