Code review comment for lp:~stub/charms/precise/postgresql/integration

The exception traceback you pastebinned is very interesting. The "user
= rel['user']" line does not exist in this branch, so I think an old
version of the charm is being deployed somehow. The actual exception
looks familiar, and I think it is one that is fixed in this branch.
Hopefully it is something simple like the trusty charm is being run
from JUJU_REPOSITORY, whereas you were hoping for the precise branch
(the test run appears to be using trusty as the series). I beat my
head against a similar wall the other day, never finding out how juju
was managing to deploy a charm that didn't exist, until I totally
rebuild my JUJU_REPOSITORY.

I can add testdep as a requirement to the makefile easily enough. Is
'charm test' no longer the proposed method of running the tests?
(done)

I don't know why SERIES is required. The makefile does "SERIES :=
$(juju get-environment default-series)" at the top, so unless it is
overridden it will be using the default series of the environment.
(unchanged - charm version issue?)

I will fix things to use the charmstore version of postgresql-psql if
the series is Precise. There is not a version in the charm store for
Trusty. Ideally, there will never be as I'd like to just include this
charm inside the PostgreSQL charm somehow (in much the same way that
Amulet launches its sentinal charms). I haven't looked at this yet, as
I've been putting it off until I have time to switch things to Amulet.
Perhaps there is some syntax that will always use the precise branch
of the postgresql-psql charm no matter if I'm deploying to trusty or
precise hosts? (done for precise)

I'd be interested in hearing thoughts on the code changes and style btw.

On 2 October 2014 00:05, Cory Johns <email address hidden> wrote:

> Even though these weren't introduced with this changeset, we really need to address them to get the postgresql charm passing in the automated test environment.

Does this block landing? Let me know (and how to go about checking),
as I've been landing other branches to this charm from other
contributers without such scrutiny.

The current branch will also fail on the automated test environment,
and it doesn't seem helpful to block on this (and it means nobody’s
changes to the charm can land, since I doubt anyone else will be
attempting to fix this). I can simply disable most of the tests if it
is a requirement (and still have more tests than most charms).

--
Stuart Bishop <email address hidden>