Merge lp://staging/~abentley/charms/precise/charmworld/auto-production into lp://staging/~juju-jitsu/charms/precise/charmworld/trunk

Proposed by Aaron Bentley
Status: Merged
Merged at revision: 34
Proposed branch: lp://staging/~abentley/charms/precise/charmworld/auto-production
Merge into: lp://staging/~juju-jitsu/charms/precise/charmworld/trunk
Diff against target: 183 lines (+112/-12)
7 files modified
common (+3/-5)
hooks/config-changed (+3/-4)
hooks/database-relation-broken (+1/-1)
hooks/database-relation-changed (+1/-1)
production_overrides.ini (+46/-0)
revision (+1/-1)
write_config.py (+57/-0)
To merge this branch: bzr merge lp://staging/~abentley/charms/precise/charmworld/auto-production
Reviewer Review Type Date Requested Status
Juju-Jitsu Hackers Pending
Review via email: mp+145420@code.staging.launchpad.net

Description of the change

Auto-generate production.ini

production.ini is now auto-generated from
1. defaults
2. overrides
3. specific settings from any existing production.ini (session.secret,
   mongo.port, mongo.host)
4. juju environment

Once production.ini is removed from the source tree, this will minimize the
chance of conflicts while ensuring production.ini is up-to-date with the
defaults.

https://codereview.appspot.com/7229057/

To post a comment you must log in.
Revision history for this message
Richard Harding (rharding) wrote :

lgtm

I wonder a little bit if we can reuse the configparser merging code abel
has in charmworld/lib/config, but I suppose it's different enough since
this code also has field limitations defined on what can be overridden.

Should there be some documentation around to help aid other people
working on the charm on how the parts fit together? If we handed this to
abel or jc would it be clear how the ini was generated and used in
production?

https://codereview.appspot.com/7229057/

Revision history for this message
Aaron Bentley (abentley) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 13-01-29 11:11 AM, <email address hidden> wrote:
> I wonder a little bit if we can reuse the configparser merging code
> abel has in charmworld/lib/config

The code in charmworld/lib/config doesn't do the merging, it delegates
that to ConfigParser.read, the same as mine does.

The bulk of charmworld/lib/config.py is concerned with emitting
warnings when sections and options are used in the override that are
not present in the default. However, this does not indicate a
problem. For example, the "logger_exc_logger" section in
production_overrides.ini is entirely legitimate, and it would be
inappropriate in the default, because the default logging
configuration is different. So that code is not useful in this context.

If I wanted to use Abel's script anyhow, it would need to take 3
inputs: defaults, production overrides, local configuration. Since it
doesn't, it's not useful here. If it did, I'd still need a Python
script. It would use ConfigParser to write out a temporary config for
local configuration.

> Should there be some documentation around to help aid other people
> working on the charm on how the parts fit together?

To me, the ideal is that the code is clear enough that documentation
is superfluous. I'm not sure whether we've reached that here.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iEYEARECAAYFAlEH+NgACgkQ0F+nu1YWqI2TVwCghD7+Rvvcornf81vhFAJZIlwR
TicAn1rcTH29iQ5IRO8yOfwfKjzD63iD
=8cOq
-----END PGP SIGNATURE-----

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: