Anna Taraday <akamyshnik...@mirantis.com> wrote: > Henry, thanks for taking care of this! > > In my opinion, it is just safe to use raw values in migration, because > migration is a strict point in time. > > I remember how many patches I send in havana in Neutron for fixing > synchronization issues. Usage constants everywhere can be good in this case, > but ModelMigrationSyc did such check of this for us already. > > If we want to have constants everywhere, we should guarantee that they are > unchanged - have test in neutron-lib which verifies their values.
Yes, we could have some tests, although a patch changing a constant would probably also have a change to the test. :) We might need to also have a prominent "Warning! Do not change this test!" comment for each test. > > > On Sat, Oct 15, 2016 at 10:41 PM Henry Gessau <hen...@gessau.net > <mailto:hen...@gessau.net>> wrote: > > Hi neutrinos, > > In Neutron many attributes are stored in database fields. The size of > these > fields therefore determines the maximum length of the attribute values. > > I would like to get some consistency in place around how we define the > constants and where they are used. Here are my thoughts... > > > 1. Raw sizes in alembic migrations > > In the alembic migrations which build the DB schema, we should use the raw > number value of the field size. > > 2. FOO_FIELD_SIZE in the sqlalchemy models > > In the sqlalchemy models, we should use the <field>_FIELD_SIZE constants > defined in neutron_lib/db/constants.py > > 3. Everywhere else, use FOO_FIELD_SIZE, or another constant (like > FOO_MAX_LEN) > based on FOO_FIELD_SIZE. > > > "Why raw numbers in alembic migrations?", you may ask. Well, we have tests > that verify that the models match the schema generated by migrations. If > both > the models and the migrations use the constants then the tests would not > detect if a patch changes the constant value. > > By using raw numbers in migrations, together with our rule of not allowing > changes to existing migrations, we allow the tests to detect and fail on > any > attempt to alter a FIELD_SIZE constant. > > Let me know if this makes sense or if you think it's a terrible idea. > > If there are no objections, I intend to submit a patch or patches to: > - replace constants with numbers in existing migrations > - ensure all models use the appropriate constants > > Existing code uses FOO_MAX_LEN in a lot of places. In most of these > places it > would make sense to simply switch to using FOO_FIELD_SIZE. However, some > code > may be quite far removed from the DB and would look better by sticking to > FOO_MAX_LEN. I added item 3. above to allow for that. __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
