Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
state: replace workers package with worker.Runner #7074
Conversation
| @@ -1,56 +1,31 @@ | ||
| -github.com/Azure/azure-sdk-for-go git 902d95d9f311ae585ee98cfd18f418b467d60d5a 2016-07-20T05:16:58Z |
| @@ -49,6 +49,8 @@ func CheckAlive(c *gc.C, w worker.Worker) { | ||
| func CheckKilled(c *gc.C, w worker.Worker) error { | ||
| wait := make(chan error, 1) | ||
| go func() { | ||
| + c.Logf("worker %T", w) |
|
Way to go @rogpeppe -1,654 lines! ;) |
| +// that is already dead and always returns the given error. | ||
| +func newDeadStoreManager(err error) *storeManager { | ||
| + var m storeManager | ||
| + m.tomb.Kill(errors.Trace(err)) |
wallyworld
Mar 15, 2017
Owner
ISTM that the callers are passing in errors.Trace(err)
So we need to just use Kill(err) here?
| +// and always returns the given error from its Err method. | ||
| +func NewDeadWatcher(err error) *Watcher { | ||
| + var w Watcher | ||
| + w.tomb.Kill(errors.Trace(err)) |
| @@ -213,7 +213,7 @@ func newResolvePendingResourceOps(pending storedResource, exists bool) []txn.Op | ||
| Resource: newRes.Resource.Resource, | ||
| id: newRes.ID, | ||
| applicationID: newRes.ApplicationID, | ||
| - lastPolled: time.Now().UTC(), | ||
| + lastPolled: time.Now().AddDate(0, 0, 0).UTC(), |
wallyworld
Mar 15, 2017
Owner
please add comment here and elsewhere to explain the AddData() usage
maybe even a todo to fix it properly
| }, | ||
| { | ||
| actionName: "juju-run", | ||
| givenPayload: map[string]interface{}{"timeout": 5 * time.Second}, | ||
| - errString: `validation failed: (root) : "command" property is missing and required, given {"timeout":5e+09}`, | ||
| + // Note: in Go 1.8 the representation of large numbers in JSON changed |
| + catacomb.Invoke(catacomb.Plan{ | ||
| + Site: &m.catacomb, | ||
| + Work: func() error { | ||
| + return errors.New("hello") |
wallyworld
Mar 15, 2017
Owner
this doesn't seem to match the doc comment
shouldn't it be "return err"?
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
rogpeppe commentedMar 8, 2017
•
Edited 1 time
-
rogpeppe
Mar 15, 2017
This fixes https://bugs.launchpad.net/juju/+bug/1669540
Instead of the custom state/workers package with, we use the existing
worker.Runner implementation. This raises the issue of what to do
when a worker cannot be acquired. The obvious solution to that is
to change the worker methods to return an error, but that turns out
to be a huge change because it means that every single Watch method
needs to return an error, and hence all the callers of those.
Instead of causing massive disruption to the code base like that, we
instead observe that watchers already have a way of returning an error -
via the Wait method. So we implement a few extra functions that can
return already-dead workers in the relevant packages (state/watcher,
state/presence and worker/lease) and use those instead.