Code review comment for lp://staging/~axwalk/juju-core/wire-up-prechecker
Revision history for this message
| William Reade (fwereade) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
5466644
demo site
(Get the code!)
Thoughts:
https:/
File state/state.go (right):
https:/
state/state.go:292: // Prechecker is a clone of environs.
here to avoid the dependency.
Better I think to define it in state, and document (in code) that
appropriate Environs implement it. environs already knows about state,
not vice versa, so we may as well just define the interface in the place
that uses it. After all, it really *doesn't* have to be just an environ
that satsisfies that interface: we'd surely want the state tests to be
written with a suitable test implementation, not of a whole environ.
https:/
state/state.go:302: }
/me strokes chin
Leaving aside the fact that we'll need lots of locking, and that makes
me a bit suspicious, this is kinda wrong; I'm worried that we would not
in general be in a position to track changes to the environment, and are
therefore in some danger of screwing up by using out-of-date
environments.
But my spidey-sense is tingling wrt the provisioner. That too needs an
up-to-date environment, and we've tied ourselves in frightful knots over
it; and, really, I don't think we need to at all. An Environ is meant to
be goroutine-safe, after all, so it might be reasonable to get a single
Environ per (authorized) agent and run a task that keeps it updated via
the API by validating changes and calling SetConfig; we can then use
that single task to keep all the environs in sync.
(I'm thinking of a Task that also implements several one-method
interfaces -- WaitPrechecker, WaitInstanceBroker, WaitPortOpenClo
-- that all share an implementation (return the single Environ held by
the task once it's available); and which just watches the API and does
the whole SetConfig lark.)
Can anyone think of a reason not to do this?
https:/