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

WIP State level WatchInterfaces #207

Merged
merged 13 commits into from Jul 2, 2014

Conversation

klyachin
Copy link

@klyachin klyachin commented Jul 1, 2014

Wraps the State level WatchInterfaces watcher, and maps the Ids we get from state into Tags for the API.

This also includes exposing the WatchInterfaces call in the API, so touching state/apiserver and state/api

wc.AssertOneChange()

// Disable the first interface.
err = ifaces[0].SetDisabled(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I would generally prefer to see positive terms than negative ones -- !disabled is a really cumbersome way to write "enabled", compared to !enabled for "disabled".

return results.Results[0].Info, nil
}

// WatchInterfaces returns a NotifyWatcher that notifies of changes of network
Copy link

Choose a reason for hiding this comment

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

s/changes of/changes to/

Copy link
Author

Choose a reason for hiding this comment

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

Done

@dimitern
Copy link

dimitern commented Jul 1, 2014

Looks almost ready, apart from a couple issues with the tests.

@dimitern
Copy link

dimitern commented Jul 1, 2014

LGTM, thank you!

@klyachin
Copy link
Author

klyachin commented Jul 1, 2014

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Jul 1, 2014

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jameinel
Copy link
Member

jameinel commented Jul 2, 2014

$$merge$$

@bz2
Copy link
Contributor

bz2 commented Jul 2, 2014

Build failed: Some weird error with not finding the landing script, probably transitory:

/tmp/hudson6827171313236674227.sh: line 121: /home/ubuntu/jenkins-github-lander/bin/lander-merge-result: No such file or directory

@bz2
Copy link
Contributor

bz2 commented Jul 2, 2014

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Jul 2, 2014

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

jujubot added a commit that referenced this pull request Jul 2, 2014
WIP State level WatchInterfaces

Wraps the State level WatchInterfaces watcher, and maps the Ids we get from state into Tags for the API.

This also includes exposing the WatchInterfaces call in the API, so touching state/apiserver and state/api
@jujubot jujubot merged commit 98fbe7b into juju:master Jul 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants