Merge lp://qastaging/~danilo/charm-helpers/unset-config-change into lp://qastaging/charm-helpers

While hookenv.config() is always passed into the Config() constructor, this dictionary does not contain items added by the charm - only those items defined in the service configuration.

The end result is that if I do hookenv.config()['whatsit_configured'] = True, then with this patch that flag will be lost in future hooks.

I suspect that to fix this that config items added by the charm will need to be tracked separately from the service config. We still need the load-config-from-saved-cache-and-tread-as-real-config, but only for new keys added by the charm and not for keys pulled in from hookenv.config() via the constructor.

Thanks for review, Stuart.

This sounds like these are two separate configuration mechanisms.

One is service configuration, and saving it is a way to track deltas. Another is charm-internal persistent configuration.

It's a shame they are bundled under the same API because we'll have to keep backwards compatibility, and the API sucks. I'll probably look into making them distinct and publicly available, and deprecate the old API (namely, hookenv.config()) that munges them together (not even sure what values should "win" in case of conflict: it makes sense for service config to override stored value when it's changed, but with current code, that's basically impossible: service config never wins over stored value).

Couple of notes:

1. Service config value always "wins" (see Config.load_previous())
2. Regarding charm-internal persistent configuration, also see `charmhelpers.core.unitdata`

I agree in hindsight that conflating the ideas of service config and arbitrary charm storage was confusing at best. charmhelpers.core.unitdata came later, and seems to be the preferred choice for simple storage within a charm, leaving the Config to be treated as read-only.

Thanks for taking a look, Tim.

Re 1, you are right, I misread that bit of code.

Re 2, since there already is a charm persistent storage with unitdata, do you have any better suggestions on how to approach this bug? Basically, as-is, 'juju unset' doesn't really propagate to a config value change, which I believe it should.

The simplest approach I see is to store service config in a different file, use that for 'change' comparison, and keep the entire config situation as is, and slowly deprecate write access to Config() objects.

Not sure this will be as seamless as I describe it (what about change notifications for values stored by the charm hooks during runtime), but sounds reasonable.