Code review comment for lp:~tvansteenburgh/charm-helpers/config-implicit-save

Revision history for this message
Tim Van Steenburgh (tvansteenburgh) wrote :

One other idea: do the save in Config.__setitem__()

I like this for two reasons:

1. No sys.exc_type voodoo
2. Keeps code local to the Config object, no coupling to or dependency on @hooks.hook

The downsides are:

1. May save() multiple times. Performance hit for writing file multiple times.
2. save()s are immediate, no rollback if exception occurs.

I think #1 is minor, and #2 is actually consistent with the way many charms have handled persisting config thus far - by writing dot files.

I think the pros outweigh the cons but would appreciate other opinions.

« Back to merge proposal