Code review comment for lp:~axwalk/juju-core/agent-setapihostports

Revision history for this message
John A Meinel (jameinel) wrote :

This feels like an incorrect layer to be doing the validation of
addresses.

I won't block it, but I'd like you to consider if the config level
should just take a list, write it down, and read it back again, and
instead the callers of this would be the ones doing the filtering.

https://codereview.appspot.com/82520043/diff/1/agent/agent.go
File agent/agent.go (right):

https://codereview.appspot.com/82520043/diff/1/agent/agent.go#newcode435
agent/agent.go:435: for _, serverHostPorts := range servers {
It feels like the wrong layer for *configInternal* to be the thing that
decides that we save CloudLocal only addresses.

Shouldn't this be done in the layer *above* config, and config is just
about "write this stuff to disk and read it back again" ?

https://codereview.appspot.com/82520043/

« Back to merge proposal