[openstack-dev] [Ceilometer] Need help with Alembic...

Boris Pavlovic boris at pavlovic.me
Tue Aug 27 17:01:00 UTC 2013


Hi,

Instead of spending tons of times through these mailing lists lets make a
code and reviews:

There is already RoadMap about what we should do.

1. Sync DB.Models with result of migrations
2. Sync migrations for different backends
3. (OSLO) Merge checker that all is synced in
https://review.openstack.org/#/c/42307/
4. (Ceilometer) add 3 in celiometer https://review.openstack.org/#/c/43872/
5. Use in TestCase class DB created by models instead of by migrations
6. (Nova then Oslo and other pr.) Run test against all backends (Mysql,
Psql)https://review.openstack.org/#/c/42142/


Then we won't need to provide sqlite in migrations and will use Alembic
without any problems.

Best regards,
Boris Pavlovic
---
Mirantis Inc.



On Tue, Aug 27, 2013 at 8:54 PM, Boris Pavlovic <boris at pavlovic.me> wrote:

> Jay,
>
>
> We are not able to use in this approach in moment  because we don't have
>> any mechanism to check that MODELS and SCHEMAS are EQUAL.
>> And actually MODELS and SCHEMAS are DIFFERENT.
>>
>
> Sorry, I don't understand the connection... how does not having a codified
> way of determining the difference between model and schema (BTW, this does
> exist in sqlalchemy-migrate... look at the compare_model_to_db method) not
> allow you to use metadata.create_all() in tests or mean that you can't run
> migrations only in production?
>
>
> There is no method out of box that will properly compare models with
> migrations..  (especially in our case of supporting alembic and
> sqlalchemy-migrate together)
>
>
>
>> 2) Sync Models and Migrations (fix DB schemas also)
>> 3) Add from oslo generic test that checks all this stuff
>> 4) Use BASE.create_all() for Schema creation instead of migrations.
>>
>
> This is already done in some projects, IIRC... (Glance used to be this
> way, at least)
>
>
> And it is totally unsafe (because result of models and migrations are
> different)
>
>
> On Tue, Aug 27, 2013 at 8:30 PM, Jay Pipes <jaypipes at gmail.com> wrote:
>
>> On 08/27/2013 04:32 AM, Boris Pavlovic wrote:
>>
>>> Jay,
>>>
>>> I should probably share to you about our work around DB.
>>>
>>> Migrations should be run only in production and only for production
>>> backends (e.g. psql and mysql)
>>> In tests we should use Schemas created by Models
>>> (BASE.metadata.create_all())
>>>
>>
>> Agree on both.
>>
>>
>>  We are not able to use in this approach in moment  because we don't have
>>> any mechanism to check that MODELS and SCHEMAS are EQUAL.
>>> And actually MODELS and SCHEMAS are DIFFERENT.
>>>
>>
>> Sorry, I don't understand the connection... how does not having a
>> codified way of determining the difference between model and schema (BTW,
>> this does exist in sqlalchemy-migrate... look at the compare_model_to_db
>> method) not allow you to use metadata.create_all() in tests or mean that
>> you can't run migrations only in production?
>>
>>
>>  E.g. in Celiometer we have BP that syncs models and migration
>>> https://blueprints.launchpad.**net/ceilometer/+spec/**
>>> ceilometer-db-sync-models-**with-migrations<https://blueprints.launchpad.net/ceilometer/+spec/ceilometer-db-sync-models-with-migrations>
>>> (in other projects we are doing the same)
>>>
>>> And also we are working around (oslo) generic tests that checks that
>>> models and migrations are equal:
>>> https://review.openstack.org/#**/c/42307/<https://review.openstack.org/#/c/42307/>
>>>
>>
>> OK, cool.
>>
>>
>>  So in our roadmap (in this case is):
>>> 1) Soft switch to alembic (with code that allows to have sqla-migrate
>>> and alembic migration in the same time)
>>>
>>
>> I don't see the point in this at all... I would rather see patches that
>> just switch to Alembic and get rid of SQLAlchemy-migrate. Create an initial
>> Alembic migration that has the last state of the database schema under
>> SQLAlchemy-migrate... and then delete SA-Migrate.
>>
>>
>>  2) Sync Models and Migrations (fix DB schemas also)
>>> 3) Add from oslo generic test that checks all this stuff
>>> 4) Use BASE.create_all() for Schema creation instead of migrations.
>>>
>>
>> This is already done in some projects, IIRC... (Glance used to be this
>> way, at least)
>>
>>  But in OpenStack is not so simple to implement such huge changes, so it
>>> take some time=)
>>>
>>>
>>> Best regards,
>>> Boris Pavlovic
>>> ---
>>> Mirantis Inc.
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> On Tue, Aug 27, 2013 at 12:02 AM, Jay Pipes <jaypipes at gmail.com
>>> <mailto:jaypipes at gmail.com>> wrote:
>>>
>>>     On 08/26/2013 03:40 PM, Herndon, John Luke (HPCS - Ft. Collins)
>>> wrote:
>>>
>>>         Jay -
>>>
>>>         It looks there is an error in the migration script that causes
>>>         it to abort:
>>>
>>>         AttributeError: 'ForeignKeyConstraint' object has no attribute
>>>         'drop'
>>>
>>>         My guess is the migration runs on the first test, creates event
>>>         types
>>>         table fine, but exits with the above error, so migration is not
>>>         "complete". Thus every subsequent test tries to migrate the db,
>>> and
>>>         notices that event types already exists.
>>>
>>>
>>>     I'd corrected that particular mistake and pushed an updated
>>>     migration script.
>>>
>>>     Best,
>>>     -jay
>>>
>>>
>>>
>>>         -john
>>>
>>>         On 8/26/13 1:15 PM, "Jay Pipes" <jaypipes at gmail.com
>>>         <mailto:jaypipes at gmail.com>> wrote:
>>>
>>>             I just noticed that every single test case for SQL-driver
>>>             storage is
>>>             executing every single migration upgrade before every single
>>>             test case
>>>             run:
>>>
>>>             https://github.com/openstack/_**_ceilometer/blob/master/__**
>>> ceilometer/tests/db.py<https://github.com/openstack/__ceilometer/blob/master/__ceilometer/tests/db.py>
>>>             <https://github.com/openstack/**ceilometer/blob/master/**
>>> ceilometer/tests/db.py<https://github.com/openstack/ceilometer/blob/master/ceilometer/tests/db.py>
>>> >
>>>             #L46
>>>
>>>             https://github.com/openstack/_**_ceilometer/blob/master/__**
>>> ceilometer/storage/imp<https://github.com/openstack/__ceilometer/blob/master/__ceilometer/storage/imp>
>>>              <https://github.com/openstack/**ceilometer/blob/master/**
>>> ceilometer/storage/imp<https://github.com/openstack/ceilometer/blob/master/ceilometer/storage/imp>
>>> >
>>>             l_sqlalchemy.py#L153
>>>
>>>             instead of simply creating a new database schema from the
>>>             models in the
>>>             current source code base using a call to
>>>             sqlalchemy.MetaData.create___**all().
>>>
>>>
>>>             This results in re-running migrations over and over again,
>>>             instead of
>>>             having dedicated migration tests that would test each
>>> migration
>>>             individually, as is the case in projects like Glance...
>>>
>>>             Is this intentional?
>>>
>>>             Best,
>>>             -jay
>>>
>>>             On 08/26/2013 02:59 PM, Sandy Walsh wrote:
>>>
>>>                 I'm getting the same problem with a different migration
>>>                 (mine is
>>>                 complaining that a column already exists)
>>>
>>>                 http://paste.openstack.org/__**show/44512/<http://paste.openstack.org/__show/44512/>
>>>
>>>                 <http://paste.openstack.org/**show/44512/<http://paste.openstack.org/show/44512/>
>>> >
>>>
>>>                 I've compared it to the other migrations and it seems
>>> fine.
>>>
>>>                 -S
>>>
>>>                 On 08/26/2013 02:34 PM, Jay Pipes wrote:
>>>
>>>                     Hey all,
>>>
>>>                     I'm trying to figure out what is going wrong with my
>>>                     code for this
>>>                     patch:
>>>
>>>                     https://review.openstack.org/_**_41316<https://review.openstack.org/__41316>
>>>
>>>                     <https://review.openstack.org/**41316<https://review.openstack.org/41316>
>>> >
>>>
>>>                     I had previously added a sqlalchemy-migrate
>>>                     migration script to add an
>>>                     event_type table, and had that working, but then was
>>>                     asked to instead
>>>                     use Alembic for migrations. So, I removed the
>>>                     sqlalchemy-migrate
>>>                     migration file and added an Alembic migration [1].
>>>
>>>                     Unfortunately, I am getting the following error when
>>>                     running tests:
>>>
>>>                     OperationalError: (OperationalError) table
>>>                     event_type already exists
>>>                     u'\nCREATE TABLE event_type (\n\tid INTEGER NOT
>>>                     NULL, \n\t"desc"
>>>                     VARCHAR(255), \n\tPRIMARY KEY (id), \n\tUNIQUE
>>>                     ("desc")\n)\n\n' ()
>>>
>>>                     The migration adds the event_type table. I've seen
>>>                     this error occur
>>>                     before when using SQLite due to SQLite's ALTER TABLE
>>>                     statement not
>>>                     allowing the rename of a column. In the
>>>                     sqlalchemy-migrate migration, I
>>>                     had a specialized SQLite migration upgrade [2] and
>>>                     downgrade [3]
>>>                     script,
>>>                     but I'm not sure how I am supposed to handle this in
>>>                     Alembic. Could
>>>                     someone help me out?
>>>
>>>                     Thanks,
>>>                     -jay
>>>
>>>                     [1]
>>>
>>>                     https://review.openstack.org/#**
>>> __/c/41316/16/ceilometer/__**storage/sqlalchemy/<https://review.openstack.org/#__/c/41316/16/ceilometer/__storage/sqlalchemy/>
>>>                     <https://review.openstack.org/**
>>> #/c/41316/16/ceilometer/**storage/sqlalchemy/<https://review.openstack.org/#/c/41316/16/ceilometer/storage/sqlalchemy/>
>>> >
>>>                     alembic/versions/49036daaaafd_**__add_event_types.py
>>>
>>>                     [2]
>>>
>>>                     https://review.openstack.org/#**
>>> __/c/41316/14/ceilometer/__**storage/sqlalchemy/<https://review.openstack.org/#__/c/41316/14/ceilometer/__storage/sqlalchemy/>
>>>                     <https://review.openstack.org/**
>>> #/c/41316/14/ceilometer/**storage/sqlalchemy/<https://review.openstack.org/#/c/41316/14/ceilometer/storage/sqlalchemy/>
>>> >
>>>                     migrate_repo/versions/013___**sqlite_upgrade.sql
>>>
>>>                     [3]
>>>
>>>                     https://review.openstack.org/#**
>>> __/c/41316/14/ceilometer/__**storage/sqlalchemy/<https://review.openstack.org/#__/c/41316/14/ceilometer/__storage/sqlalchemy/>
>>>                     <https://review.openstack.org/**
>>> #/c/41316/14/ceilometer/**storage/sqlalchemy/<https://review.openstack.org/#/c/41316/14/ceilometer/storage/sqlalchemy/>
>>> >
>>>                     migrate_repo/versions/013___**sqlite_downgrade.sql
>>>
>>>
>>>                     ______________________________**___________________
>>>                     OpenStack-dev mailing list
>>>                     OpenStack-dev at lists.openstack.**__org
>>>                     <mailto:OpenStack-dev at lists.**openstack.org<OpenStack-dev at lists.openstack.org>
>>> >
>>>                     http://lists.openstack.org/__**
>>> cgi-bin/mailman/listinfo/__**openstack-dev<http://lists.openstack.org/__cgi-bin/mailman/listinfo/__openstack-dev>
>>>                     <http://lists.openstack.org/**
>>> cgi-bin/mailman/listinfo/**openstack-dev<http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev>
>>> >
>>>
>>>
>>>
>>>             ______________________________**___________________
>>>             OpenStack-dev mailing list
>>>             OpenStack-dev at lists.openstack.**__org
>>>             <mailto:OpenStack-dev at lists.**openstack.org<OpenStack-dev at lists.openstack.org>
>>> >
>>>             http://lists.openstack.org/__**cgi-bin/mailman/listinfo/__**
>>> openstack-dev<http://lists.openstack.org/__cgi-bin/mailman/listinfo/__openstack-dev>
>>>             <http://lists.openstack.org/**cgi-bin/mailman/listinfo/**
>>> openstack-dev<http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev>
>>> >
>>>
>>>
>>>             ______________________________**___________________
>>>             OpenStack-dev mailing list
>>>             OpenStack-dev at lists.openstack.**__org
>>>             <mailto:OpenStack-dev at lists.**openstack.org<OpenStack-dev at lists.openstack.org>
>>> >
>>>             http://lists.openstack.org/__**cgi-bin/mailman/listinfo/__**
>>> openstack-dev<http://lists.openstack.org/__cgi-bin/mailman/listinfo/__openstack-dev>
>>>             <http://lists.openstack.org/**cgi-bin/mailman/listinfo/**
>>> openstack-dev<http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev>
>>> >
>>>
>>>
>>>
>>>     ______________________________**___________________
>>>     OpenStack-dev mailing list
>>>     OpenStack-dev at lists.openstack.**__org
>>>     <mailto:OpenStack-dev at lists.**openstack.org<OpenStack-dev at lists.openstack.org>
>>> >
>>>     http://lists.openstack.org/__**cgi-bin/mailman/listinfo/__**
>>> openstack-dev<http://lists.openstack.org/__cgi-bin/mailman/listinfo/__openstack-dev><
>>> http://lists.openstack.org/**cgi-bin/mailman/listinfo/**openstack-dev<http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev>
>>> >
>>>
>>>
>>>
>>>
>>>
>>> ______________________________**_________________
>>> OpenStack-dev mailing list
>>> OpenStack-dev at lists.openstack.**org <OpenStack-dev at lists.openstack.org>
>>> http://lists.openstack.org/**cgi-bin/mailman/listinfo/**openstack-dev<http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev>
>>>
>>>
>>
>> ______________________________**_________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.**org <OpenStack-dev at lists.openstack.org>
>> http://lists.openstack.org/**cgi-bin/mailman/listinfo/**openstack-dev<http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130827/f5f66c2a/attachment-0001.html>


More information about the OpenStack-dev mailing list