Code review comment for lp:~frankban/juju-quickstart/new-bootstrap-strategy

Revision history for this message
Francesco Banconi (frankban) wrote :

Thanks for the reviews, really good stuff!

https://codereview.appspot.com/172380043/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/172380043/diff/1/quickstart/app.py#newcode190
quickstart/app.py:190: candidates = jenv.get_value(env_name,
'state-servers')
On 2014/11/12 15:30:44, rharding wrote:
> do you know how this list gets updated from Juju? In an HA situation
does juju
> rewrite the jenv file?

I don't know that for sure: it could be the case when you set HA from
the CLI. I would be surprised if the jenv is updated when you will set
HA using the API. In case of stale addresses, we just fall back to the
old bootstrap strategy.

https://codereview.appspot.com/172380043/diff/1/quickstart/app.py#newcode245
quickstart/app.py:245: # For this reason, a call to status() is usually
required at this point.
On 2014/11/12 14:48:25, bac wrote:
> This is no different from the calling perspective than the 'not
already
> bootstrapped' situation, right?

> If so, perhaps that should be noted in the docstring for this
function.

Done.

https://codereview.appspot.com/172380043/diff/1/quickstart/netutils.py
File quickstart/netutils.py (right):

https://codereview.appspot.com/172380043/diff/1/quickstart/netutils.py#newcode50
quickstart/netutils.py:50: timeout = 3
On 2014/11/12 14:48:25, bac wrote:
> Make timeout an arg with default = 3?

Done.

https://codereview.appspot.com/172380043/

« Back to merge proposal