Enable reading and setting machine owner data #57

Merged
merged 4 commits into from Sep 8, 2016

Conversation

Projects
None yet
5 participants
Member

babbageclunk commented Sep 8, 2016

Expose owner data from the API - this is key/value storage that can be set by the owner of the machine and is cleared when the machine is released.

It's needed so Juju can to store controller/model information for instances without using the Storage interface.

controller_test.go
+ },
+ }
+ err := args.Validate()
+ c.Assert(err, gc.ErrorMatches, "searching by tags with other filters not supported")
@frobware

frobware Sep 8, 2016

Is there a test for hasOtherFields?

@babbageclunk

babbageclunk Sep 8, 2016

Member

Sorry, there was a bit of churn in this change (because I didn't know about set-owner-data I was faking it horribly using tags) - this change is removed in a subsequent commit.

Contributor

voidspace commented Sep 8, 2016

LGTM

machine.go
+ if field == nil {
+ return nil
+ }
+ fieldMap := field.(map[string]interface{})
@dimitern

dimitern Sep 8, 2016

Contributor

If there's ever a chance for field to have a different type, this will panic.
But I suspect you can only call this fields passed through the schema validation/coercion, so should be fine (could use a comment though - anyone looking at it later might wonder why don't we care about panicking).

@babbageclunk

babbageclunk Sep 8, 2016

Member

Yes, it's always guarded by schema coercion - I'll put in a comment to that effect.

testing.go
func (s *SimpleTestServer) RequestCount() int {
return len(s.requests)
}
+func (s *SimpleTestServer) ResetRequests() {
+ s.requests = []*http.Request{}
@dimitern

dimitern Sep 8, 2016

Contributor

Any reason to not just use s.requests = nil here?

@babbageclunk

babbageclunk Sep 8, 2016

Member

Nope! Brainfart - I'll change it.

Contributor

dimitern commented Sep 8, 2016

LGTM, but I have a couple of questions.

Member

babbageclunk commented Sep 8, 2016

$$merge$$

Contributor

jujubot commented Sep 8, 2016

@jujubot jujubot merged commit 5235db1 into juju:master Sep 8, 2016

@babbageclunk babbageclunk deleted the babbageclunk:tags branch Sep 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment