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: refactor Open to take OpenParams #6520
Conversation
anastasiamac
approved these changes
Nov 1, 2016
LGTM \o/
Please clarify what happens if dial-opts is nil: either there is some default or it is not valid.
Apart from ^^ PR looks amazing!
| @@ -298,7 +299,13 @@ LXC_BRIDGE="ignored"`[1:]) | ||
| info, ok := cfg.MongoInfo() | ||
| c.Assert(ok, jc.IsTrue) | ||
| c.Assert(info.Password, gc.Not(gc.Equals), testing.DefaultMongoPassword) | ||
| - st1, err := state.Open(newCfg.Model(), newCfg.Controller(), info, mongotest.DialOpts(), nil) | ||
| + st1, err := state.Open(state.OpenParams{ |
| + | ||
| + // MongoDialOpts is the mongo.DialOpts used to control how | ||
| + // Mongo connections are made. | ||
| + MongoDialOpts mongo.DialOpts |
anastasiamac
Nov 1, 2016
Member
Please either comment what would happen if dialopts are nil or add to validation to ensure that it does not happen.
| -) (*State, error) { | ||
| - st, err := open(controllerModelTag, info, opts, newPolicy, clock.WallClock) | ||
| +func Open(args OpenParams) (*State, error) { | ||
| + if err := args.Validate(); err != nil { |
|
$$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 |
jujubot
merged commit 04a449b
into
juju:develop
Nov 2, 2016
1 check passed
github-check-merge-juju
Built PR, ran unit tests, and tested LXD deploy.
Details
added a commit
that referenced
this pull request
Nov 16, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
axw commentedNov 1, 2016
Introduce a state.OpenParams struct, and
update state.Open to use it. Add a Clock
parameter while we're at it.
A followup will introduce an observer for
recording mgo/txn-related metrics.