Code review comment for lp:~axwalk/juju-core/wire-up-prechecker-take2

Revision history for this message
Andrew Wilkins (axwalk) wrote :

On 2014/02/17 10:00:17, fwereade wrote:
> possible approach question, please discuss with waigani:

> the issue with setting a prechecker, even if it's watched, is
raciness. I was
> willing to handwave it a bit before, but now we have waigani's use
case which
> requires that we get *less* racy wrt environ config... how about
setting not a
> prechecker, but something like a getter: eg
`func(map[string]interface{})
> (Prechecker, error)`? This is irrelevant for your purposes really [0]
but
> critically important for waigani's -- and the only difference is in
the type
> that needs to be returned (whether it has SetConfig or Precheck).
That'd need a
> bit of finesse but it'd let you use basically the same implementation
for
> basically the same task -- *and* mean that you don't need to do the
watching
> followup (at the cost of an extra db hits when prechecking -- worth
it, I
> think).

I might just create one getter type that can do Precheck or Validate,
but otherwise I like this alternative. I'll look into implementing this,
and discuss with waigani in our standup tomorrow.

> *and* we wouldn't need to mess around with locks in state :).

> [0] you *could* use it to assert that the environ hasn't changed since
you
> checked validity of the upcoming op, but the value of that is somewhat
limited I
> think

Yeah, I don't think we need to go that far.

https://codereview.appspot.com/61520045/

« Back to merge proposal