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
> 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
> 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
> 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
> 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
> 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)