Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Expose machine.MarkForRemoval in provisioner API #6042
Conversation
fwereade
reviewed
Aug 22, 2016
| + c.Assert(results, gc.HasLen, 6) | ||
| + c.Check(results[0].Error, gc.IsNil) | ||
| + c.Check(*results[1].Error, gc.Equals, | ||
| + *common.ServerError(errors.NotFoundf("machine 100"))) |
fwereade
Aug 22, 2016
Contributor
Explicitly checking the code (IsCodeNotFound?) is probably best here -- that's what any consumer will actually be responding to. (It's also nice to check the message too, but the code is the really important thing.)
dimitern
Aug 22, 2016
Contributor
+1
c.Check(results[1].Error, jc.Satisfies, params.IsCodeNotFound)(IIRC)
|
It would be really nice if these weren't full-stack tests, but we can't fix everything at once. LGTM with trivially tweaked error-checking. |
|
LGTM as well |
babbageclunk
added some commits
Aug 18, 2016
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
Ugh, fixed stupid typo. Could've sworn I'd rerun those tests! |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
babbageclunk commentedAug 19, 2016
Part of lp:1585878
Once the machine undertaker is ready the provisioner task will mark machines for removal instead of removing them directly.
From talking to thumper, there's no need to bump the version of the provisioner facade as long as this gets into the beta.