yellow team mailing list archive

[Nicola Larosa <nicola.larosa@xxxxxxxxxxxxx>]

Thanks for the review, very useful.


https://codereview.appspot.com/7007045/diff/6002/hooks/config-changed
File hooks/config-changed (right):

https://codereview.appspot.com/7007045/diff/6002/hooks/config-changed#newcode88
hooks/config-changed:88: start_improv(
frankban wrote:
> The improv and the api agent services now depend on the certificates
> path, and need to be restarted if the ssl-cert-path is changed.
> AFAICT, currently this is not handled here.

You are right, I will add ssl-cert-path to the checked properties.

https://codereview.appspot.com/7007045/diff/6002/hooks/config-changed#newcode106
hooks/config-changed:106: start_agent(
frankban wrote:
> See above.

Ditto.

https://codereview.appspot.com/7007045/diff/6002/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/7007045/diff/6002/hooks/utils.py#newcode59
hooks/utils.py:59: SSL_CERT_PATH = '/etc/ssl/juju-gui'
frankban wrote:
> I believe that the default value is never used, i.e. each time we call
> start_improv, start_agent etc., we always pass the value taken from
config.

Indeed. I foolishly unified the defaults into the constant, forgetting
that the value comes from the config anyway, and the defaults do not
really make sense. I'll remove them.

https://codereview.appspot.com/7007045/diff/6002/hooks/utils.py#newcode339
hooks/utils.py:339: crt_path = os.path.join(ssl_cert_path, 'juju.crt')
frankban wrote:
> I like how you renamed the keys.

Well, I had to. :-) The --keys Juju option only passes the certificates
directory, not the filenames: they are hardwired.

https://codereview.appspot.com/7007045/

-- 
https://code.launchpad.net/~teknico/charms/precise/juju-gui/encrypt-api-env-connection/+merge/141107
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~teknico/charms/precise/juju-gui/encrypt-api-env-connection into lp:~juju-gui/charms/precise/juju-gui/trunk.