Code review comment for lp:~johnsca/charms/trusty/cf-cloud-controller/refactor

Revision history for this message
Alex Lomov (lomov-as) wrote :

https://codereview.appspot.com/91450050/diff/20001/hooks/install
File hooks/install (right):

https://codereview.appspot.com/91450050/diff/20001/hooks/install#newcode60
hooks/install:60: common.FOG_CONNECTION,
I can't get why you called it common and not config, and why we can't
have separate file for all this config options to parse it using
ConfigParser or similar tool.

https://codereview.appspot.com/91450050/diff/20001/hooks/install#newcode69
hooks/install:69: common.db_migrate()
I saw you added mysql relation to metadata.yml. It will be really cool
to have opportunity to connect it. Yay! But how and when are you going
create/migrate database for it? Also the question with config template
is still open.

https://codereview.appspot.com/91450050/diff/20001/templates/cloud_controller.yml
File templates/cloud_controller.yml (right):

https://codereview.appspot.com/91450050/diff/20001/templates/cloud_controller.yml#newcode57
templates/cloud_controller.yml:57: database: {{db['dsn']}}
Is it the last version? I'm not 100% sure it will work with changes in
relations. how do you plan to connect mysql relation here?

https://codereview.appspot.com/91450050/

« Back to merge proposal