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.

Thanks! I restored the check in changed.

>
> > 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.

Ok, let me know if anything else is needed.

Cheers,

Diogo