Code review comment for lp:~nick-moffitt/charms/precise/keystone/migrate-sqlite-to-mysql

Revision history for this message
Nick Moffitt (nick-moffitt) wrote :

Adam, thanks for helping to test this!

In my initial use case, we had a system that we realized with some shock had been running with sqlite, and we were about to see an increase in the volume of the database. Since this charm was already running on a production system, I needed to make sure that the new package dependencies (sqlfairy itself, for instance) were installed before the relation hook fired.

In my understanding, no configuration steps should be taken in install hooks, since a fresh install will get a config-changed after the install hook fires anyway. Unfortunately this charm seemed to have done some config steps in the install hook, so I was nervous about trusting its idempotence and calling it from my upgrade hook.

Since this was meant to roll out a change to our production service, I took the simplest step forward that would obviously work correctly. So that's why there's duplication of the apt-get steps in the upgrade hook function.

I would love to see that bit detangled somewhat, but it seemed out of scope for this merge.

« Back to merge proposal