Code review comment for lp:~matsubara/charms/precise/oops-tools/trunk

Excerpts from Diogo Matsubara's message of 2012-07-02 08:40:28 UTC:
> > Diogo, this does not look right.
>
> Hi Clint, thanks for the review
>
> >
> > I see config-get's mixed with relation-get's in db-relation-changed/joined.
>
> The way I understand the workflow is:
>
> db-relation-joined sets the values for USER and DATABASE from the config values and pass those to the postgresql charm
> Once postgresql charm finishes doing its job, it triggers the db-relation-changed and oops-tools sets its configs from those values. Before, oops-tools charm didn't have a db-relation-joined hook, so that's why there was a check if the user was set. Now that it has a joined and changed hooks, when the changed runs I can safely assume the db charm has been run.

Ahh, no you cannot make this assumption. joined and changed *always*
run in series at the beginning of a relationship. You cannot assume that
the value has been set when changed runs, as it may just be running in
that initial first-pass mode before relation-set has happened on the
other side. It will be run again when the values are set.

> Is this a misunderstanding on my part? Please help clarify so I can fix this.
>
> >
> > There is a rabbitmq charm, so I'd like to see a rabbitmq relation in addition
> > to the config.yaml values for the external AMQP server.
> >
>
> Not sure I understand this, I added a amqp-relation-changed hook. What else is missing?

I think I just must have missed that. Will review again once the check
for empty relation values is restored.