I don't have a problem with any of these requirements, but I'd like to explore automating the checks. Would it be possible to write a unit test that verified this for all migrations? Then we don't need to add it to the checklist...
Michael On Thu, Dec 19, 2013 at 4:27 AM, Matt Riedemann <mrie...@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 > > -- > > Thanks, > > Matt Riedemann > > > _______________________________________________ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Rackspace Australia _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
