-
Notifications
You must be signed in to change notification settings - Fork 36
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
controller: Accept a versioned API URL #65
Conversation
This enables creating a controller without doing version probing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General approval. Just one additional test requested. Also thinking on how we can avoid treating transient network errors the same as unsupported versions.
client_test.go
Outdated
expectedURL, err := url.Parse("http://example.com/MAAS/api/1.0/") | ||
c.Assert(err, jc.ErrorIsNil) | ||
func (suite *ClientSuite) TestAddAPIVersionToURL(c *gc.C) { | ||
apiurl := AddAPIVersionToURL("http://example.com/MAAS", "1.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also add a test for the base url having a trailing slash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wilco.
case err == nil: | ||
return controller, nil | ||
case IsBadVersionInfoError(err): | ||
// TODO(babbageclunk): this is bad - it treats transient |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can we treat transient network errors differently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure - I was leaving that there for myself when I work on the next bug.
If a versioned url is used, the normal version negotiation will be skipped.
f57a1dd
to
c8e3e67
Compare
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju-gomaasapi |
At the moment if someone tries to specify a particular API version in the URL (e.g.
http://maas.server/MAAS/api/2.0/
), we try to tack the version on the end again and fail to connect. Handle this by detecting the version in the URL and skipping the normal loop over supported versions.Fix for this Juju bug: https://bugs.launchpad.net/juju/+bug/1675485