Uniter API now has WatchActions #141
Conversation
| // Initial event. | ||
| wc.AssertChange() | ||
|
|
||
| // Update config a couple of times, check a single event. |
|
This is looking good - just a few concerns about test coverage of error cases. |
|
|
||
| // Consume the initial event. API calls to Watch 'transmit' | ||
| // the initial event in the Watch response. | ||
| if _, ok := <-watch.Changes(); ok { |
There was a problem hiding this comment.
The discarded result ought to be sent back in the response...
|
This is looking good. All issues I've raised have been addressed. It might be best to wait for a final pass by @fwereade before this lands. |
| Changes: changes, | ||
| }, nil | ||
| } | ||
| return nothing, watcher.MustErr(watch) |
There was a problem hiding this comment.
Hmm, that MustErr is a problem. Is it the same everywhere? Even if a single watcher is somehow screwed up, it shouldn't be able to take out the whole API server...
Wouldn't surprise me if we had the damn things everywhere, so don't feel obliged if it's consistent -- but please do add a bug pointing out that MustErr is not safe and should basically never be used except in client code.
There was a problem hiding this comment.
https://bugs.launchpad.net/juju-core/+bug/1334390 has been opened.
|
I've updated this to reflect @johnweldon's changes to the ActionsWatcher behavior (Actions pending at the time of the Watcher's creation now arrive with the first event.) |
| @@ -437,6 +437,26 @@ func (u *Unit) WatchAddresses() (watcher.NotifyWatcher, error) { | |||
| return w, nil | |||
| } | |||
|
|
|||
| func (u *Unit) WatchActions() (watcher.StringsWatcher, error) { | |||
There was a problem hiding this comment.
public method needs a comment
This commit adds state/api and state/apiserver endpoints for WatchActions.
…as appropriate results when actions go through
|
LGTM. |
|
|
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
Uniter API now has WatchActions This commit adds state/api and state/apiserver endpoints for WatchActions.
Linkify site
This commit adds state/api and state/apiserver endpoints for
WatchActions.