Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Add new offer status watcher facade #7955
Conversation
| + } | ||
| + changes := copyChanges(initialChanges) | ||
| + for { | ||
| + select { |
axw
Oct 24, 2017
Member
apparently I missed it when reviewing the relation status watcher client, but this code is preventing client-side coalescence
you should have a single select to allow coalescing of offer statuses, like:
out := w.out
changes := copyChanges(initialChanges)
for {
select {
case <-w.tomb.Dying():
return tomb.ErrDying
case data, ok := <-w.in:
if !ok {...}
changes = combineChanges(changes, copyChanges(data...))
out = w.out
case out <- changed:
out = nil
changes = nil
}
}
| + }) | ||
| + c.Assert(err, jc.ErrorIsNil) | ||
| + stop := func() { | ||
| + c.Assert(worker.Stop(w), jc.ErrorIsNil) |
| + results.Results[i].Error = common.ServerError(watcher.EnsureErr(w)) | ||
| + continue | ||
| + } | ||
| + results.Results[i].OfferStatusWatcherId = api.resources.Register(w) |
| + results.Results[i].OfferStatusWatcherId = api.resources.Register(w) | ||
| + change, err := commoncrossmodel.GetOfferStatusChange(api.st, arg.OfferUUID) | ||
| + if err != nil { | ||
| + results.Results[i].Error = common.ServerError(err) |
|
Thanks for adding the coalescence test! |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju |
jujubot
merged commit 2003b94
into
juju:develop
Oct 24, 2017
1 check passed
continuous-integration/jenkins/pr-merge
This commit looks good
Details
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
wallyworld commentedOct 23, 2017
Description of change
Add a new facade to support watching the status of offers. As is usual with this type of change, there's a lot of boilerplate. Subsequent work will make use of this new functionality.
QA steps
Run the tests - there's nothing to test yet until the next branch.