Open Stack

On 12/18/2013 1:14 PM, Brant Knudson wrote:
> Matt -
>
> Could a test be added that goes through the models and checks these
> things? Other projects could use this too.
>
> Here's an example of a test that checks if the tables are all InnoDB:
> http://git.openstack.org/cgit/openstack/nova/tree/nova/tests/db/test_migrations.py?id=6e455cd97f04bf26bbe022be17c57e089cf502f4#n430
>
> - Brant
>

Brant, I could see automating #3 since you could trace the FK to the UC 
and then check if the columns in the UC are nullable or not, but I'm not 
sure how easy it would be to generically test 1 and 2 because we don't 
have strict naming conventions on UC/FK names as far as I know, but I 
guess we could start enforcing that with a test, and whitelist any 
existing UC/FK names that don't fit the new convention.

Thoughts on that or other ideas how to automate checking for this?

>
>
>
> On Wed, Dec 18, 2013 at 11:27 AM, Matt Riedemann
> <mriedem at linux.vnet.ibm.com <mailto:mriedem at linux.vnet.ibm.com>> wrote:
>
>     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.
>
>     [1] https://wiki.openstack.org/__wiki/ReviewChecklist
>     <https://wiki.openstack.org/wiki/ReviewChecklist>
>
>     --
>
>     Thanks,
>
>     Matt Riedemann
>
>
>     _________________________________________________
>     OpenStack-dev mailing list
>     OpenStack-dev at lists.openstack.__org
>     <mailto: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
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>

-- 

Thanks,

Matt Riedemann