Open Stack

On Wed, Sep 04 2013, Jay Pipes wrote:

Hi Jay,

> So I went to do the work I said I was going to do at last week's Ceilometer
> meeting -- translate the 2 Alembic migrations in the Ceilometer source into
> SA-migrate migrations -- and then rebased my branch only to find 2 more
> Alembic migrations added in the last few days:
>
> https://review.openstack.org/#/c/42716/
> https://review.openstack.org/#/c/42715/
>
> I will note that there is no unit testing of either of these migrations,
> because neither of them runs on SQLite, which is what the unit tests use
> (improperly, IMHO).

Agreed. That's the reason I jumped in and submitted
https://review.openstack.org/#/c/44681/ to add devstack-gate to
Ceilometer, so we can catch this in the future.

I'm sorry these got in in the mean time, I didn't think about what you
were working on when pushing the button and that it would affect you.

> There is a unique constraint name in one of them (only
> apparently used in the PostgreSQL driver) that is inconsistent with the
> naming of unique constraints that is used in the other migration. Note that
> I am not in favor of the unique constraint naming convention of
> table_columnA0columnB0columnC0, as I've noted in the upstream oslo.db patch
> that adds a linter-style check for this convention:
>
> https://review.openstack.org/#/c/42307/2

Noted. I thought there was already some sort of convention around this.

> I thought we were going to translate the existing 2 Alembic migrations to
> SA-migrate migrations, and then do a switch to Alembic (removing the old
> SA-migrate versioning) in Icehouse-1? This was supposed to get us past the
> current mess of having both SA-migrate and Alembic migrations in the same
> source code base -- which is confusing a bunch of contributors who have
> written SA-migrate migrations.
>
> Can we have a decision on this please?

That was my understanding too, as I've also written a new migration
using SA-migrate.

> I thought the plan from last week was:
>
> 1) Translate the 2 Alembic migrations to SA-Migrate migrations
> 2) Remove Alembic support from Ceilometer
> 3) Add unit tests (pretty much as-is from Glance) that would test the
> SA-migrate migrations in the unit tests as well as the MySQL and PostgreSQL
> testers in the gate
> 4) Add SA-migrate migrations for the remainder of Havana
> 5) Immediately after the cut of Havana final, do a cutover to Alembic from
> SA-migrate that would:
>  a) Create an initial Alembic migration that would be the schema state of
> the Ceilometer database at the last cut of Havana
>  b) Write a simple check for the migrate_version table in the database to
> check if the database was under SA-migrate control. If so, do nothing other
> than remove the migrate_version table
>  c) Remove all the ceilometer/storage/sqlalchemy/migrate_repo/*

Sounds good to me.

Now we need to have the PostgreSQL migration fixed one way or another.
Svetlana wrote https://review.openstack.org/#/c/44539/ and I wrote
https://review.openstack.org/#/c/44691/ which try to fix the harm done.

I think the best call is to drop all of these and let your patch goes
in.

-- 
Julien Danjou
// Free Software hacker / independent consultant
// http://julien.danjou.info
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130904/34ec2a29/attachment.pgp>