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

Revision history for this message
William Reade (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).

*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

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

« Back to merge proposal