Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
apiserver: provide constraints as string to Application.(Deploy|Update). #6639
Conversation
|
!!build!! |
|
!!build!! |
|
!!tryagain!! |
kat-co
reviewed
Dec 1, 2016
I think we should either support passing a constraints.Constraints object over the wire, or a constraints-string, but not both. I like the idea of supporting a string because that centralizes the parsing for all clients to the server.
This necessitates removing the constraints parsing from all existing commands and simply passing through the string.
Also -- and apologies ahead of time -- we need to at least make an effort to write unit tests instead of full-stack tests. What can we do to write your new tests in-memory?
| @@ -1094,7 +1094,77 @@ func (s *serviceSuite) TestServiceDeployConfigError(c *gc.C) { | ||
| c.Assert(err, jc.Satisfies, errors.IsNotFound) | ||
| } | ||
| -func (s *serviceSuite) TestServiceDeployToMachine(c *gc.C) { | ||
| +func (s *serviceSuite) TestApplicationDeployConstraints(c *gc.C) { |
kat-co
Dec 1, 2016
Contributor
Please also include some idea of expected outcome in the name of your tests. Style is up to you, but I prefer this: TestAppDeployConstraints_PicksCorrectType
| + }}}) | ||
| + c.Assert(err, jc.ErrorIsNil) | ||
| + c.Assert(results.Results, gc.HasLen, 1) | ||
| + c.Assert(results.Results[0].Error, gc.IsNil) |
| + c.Assert(err, jc.ErrorIsNil) | ||
| + obtained, err := application.Constraints() | ||
| + c.Assert(err, jc.ErrorIsNil) | ||
| + c.Assert(obtained, gc.DeepEquals, cons) |
kat-co
Dec 1, 2016
Contributor
Anything that doesn't preclude the rest of the test from running should be c.Check (even if it is currently the last statement in the test).
|
Closing this, as the problem must be solved with another, completely different approach. |
frankban commentedNov 30, 2016
This way clients are not required to reimplement the constraints.Parse logic.
This fixes https://bugs.launchpad.net/juju/+bug/1645402 in a API backward compatible way.