Open Stack

On 12/18/2013 2:11 PM, Dan Prince wrote:
>
>
> ----- Original Message -----
>> From: "Matt Riedemann" <mriedem at linux.vnet.ibm.com>
>> To: "OpenStack Development Mailing List (not for usage questions)" <openstack-dev at lists.openstack.org>
>> Sent: Wednesday, December 18, 2013 12:27:49 PM
>> Subject: [openstack-dev] Adding DB migration items to the common review	checklist
>>
>> I've seen this come up a few times in reviews and was thinking we should
>> put something in the general review checklist wiki for it [1].
>>
>> Basically I have three things I'd like to have in the list for DB
>> migrations:
>>
>> 1. Unique constraints should be named. Different DB engines and
>> SQLAlchemy dialects automatically name the constraint their own way,
>> which can be troublesome for universal migrations. We should avoid this
>> by enforcing that UCs are named when they are created. This means not
>> using the unique=True arg in UniqueConstraint if the name arg isn't
>> provided.
>>
>> 2. Foreign keys should be named for the same reasons in #1.
>>
>> 3. Foreign keys shouldn't be created against nullable columns. Some DB
>> engines don't allow unique constraints over nullable columns and if you
>> can't create the unique constraint you can't create the foreign key, so
>> we should avoid this. If you need the FK, then the pre-req is to make
>> the target column non-nullable. Think of the instances.uuid column in
>> nova for example.
>>
>> Unless anyone has a strong objection to this, I'll update the review
>> checklist wiki with these items.
>
> No objection to these.
>
> One possible addition would be to make sure that migrations stand on their own as much as possible. Code sharing, while good in many cases, can bite you in DB migrations because fixing a bug in the shared code may change the behavior of an old (released) migration. So by sharing migration code it then can  become easier to break upgrades paths down the road. If we make some exceptions to this rule with nova.db.sqlalchemy we need to be very careful that we don't change the behavior in those functions. Automated tests help here too.
>
> Dan

Not sure if this is pure coincidence, but case in point:

https://review.openstack.org/#/c/62965/

>
>>
>> [1] https://wiki.openstack.org/wiki/ReviewChecklist
>>
>> --
>>
>> Thanks,
>>
>> Matt Riedemann
>>
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>

-- 

Thanks,

Matt Riedemann