Skip to content

Expand API with new Machine, Interface, and Tags interfaces#85

Open
seanhoughton wants to merge 21 commits into
juju:masterfrom
seanhoughton:bonds
Open

Expand API with new Machine, Interface, and Tags interfaces#85
seanhoughton wants to merge 21 commits into
juju:masterfrom
seanhoughton:bonds

Conversation

@seanhoughton
Copy link
Copy Markdown

This PR adds a bunch of functionality. I haven't spent any time adding unit tests for the new functions or learning how the unit testing framework is set up, but it would be great if you could take a look and offer any suggestions or critiques before I spend any time on that.

This PR also adds a go module setup.

@jujubot
Copy link
Copy Markdown
Contributor

jujubot commented Jan 31, 2020

Can one of the admins verify this patch?

1 similar comment
@jujubot
Copy link
Copy Markdown
Contributor

jujubot commented Jan 31, 2020

Can one of the admins verify this patch?

@mitechie
Copy link
Copy Markdown

Thanks, we'll see if we can get a look across things.

!!build!!

Comment thread client.go
if err != nil {
return nil, err
}
for retry := 0; retry < NumberOfRetries; retry++ {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Curious, what dies the hashicorp retryable client offer?

Comment thread client.go
httpClient := retryablehttp.NewClient()

// Need to re-sign the request before each retry
httpClient.RequestLogHook = func(logger retryablehttp.Logger, request *http.Request, count int) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We'd need to adapt a github.com/juju/loggo logger as the log writer implementation

Comment thread tag.go
return s.definition
}

func (s *tag) Machines() ([]Machine, error) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Another way of doing this is to keep the tag object as a pure domain object and have the api to get machines for a given tag as a method on the controller (or even as a method on the machine). A tag is really just a snippet of domain information. Same with AddToMachine() etc; they perhaps are better done as AddTag() and RemoveTag() methods on the machine entity.

Comment thread tag.go
@@ -0,0 +1,150 @@
package gomaasapi
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs a copyright header

Comment thread controller.go
return result, nil
}

func (c *controller) GetMachine(systemID string) (Machine, error) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

public methods/structs etc should have a comment

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.

5 participants