https://codereview.appspot.com/14032043/diff/1/state/state.go#newcode292
state/state.go:292: // Prechecker is a clone of environs.Prechecker,
here to avoid the dependency.
On 2013/09/27 10:59:22, fwereade wrote:
> 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.
Fair enough, I'll do that. Do you think it'd be worthwhile adding
something like below to environs/interface.go?
// Prechecker is an optional Environ interface. See state.Prechecker for
details.
type Prechecker interface {
state.Prechecker
}
> 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, WaitPortOpenCloseThing -- 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?
That sounds sensible to me, though I don't know the finer points of the
machine agent.
I lbox proposed with -wip, so this wouldn't have circulated; "anyone" is
just me at the moment.
https:/ /codereview. appspot. com/14032043/ diff/1/ state/state. go
File state/state.go (right):
https:/ /codereview. appspot. com/14032043/ diff/1/ state/state. go#newcode292 Prechecker,
state/state.go:292: // Prechecker is a clone of environs.
here to avoid the dependency.
On 2013/09/27 10:59:22, fwereade wrote:
> 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.
Fair enough, I'll do that. Do you think it'd be worthwhile adding interface. go?
something like below to environs/
// Prechecker is an optional Environ interface. See state.Prechecker for Prechecker
details.
type Prechecker interface {
state.
}
https:/ /codereview. appspot. com/14032043/ diff/1/ state/state. go#newcode302
state/state.go:302: }
On 2013/09/27 10:59:22, fwereade wrote:
> /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 seThing -- that all
interfaces --
> WaitPrechecker, WaitInstanceBroker, WaitPortOpenClo
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?
That sounds sensible to me, though I don't know the finer points of the
machine agent.
I lbox proposed with -wip, so this wouldn't have circulated; "anyone" is
just me at the moment.
https:/ /codereview. appspot. com/14032043/