Only allow alpha-2 codes for countries

[liZe]

This PR is the end of #113 and endlessm/eos-activation-server#23. This branch has been forgotten for too long (sorry), it’s time to merge it!

(Just in case: I’ve checked that there’s no alpha-3 code left in the database.)

https://phabricator.endlessm.com/T29210

[wjt]

Tests fail:

>       cursor.execute(statement, parameters)

E       psycopg2.errors.CheckViolation: new row for relation "activation_v1" violates check constraint "ck_activation_v1_country_code_2_chars"

E       DETAIL:  Failing row contains (1, unknown, vendor, product, release, null, null, null, null, FRA, null, null, null, null, 2020-10-16 13:35:28.77033+00, null, null, null, null, null, null).

[liZe]

Tests fail:

My bad. The master branch contains the 3-to-2 migration commands whose tests fail, now that the table is 2-letter only. I have merge master and then remove the commands and their tests.

[liZe]

My bad. The master branch contains the 3-to-2 migration commands whose tests fail, now that the table is 2-letter only. I have merge master and then remove the commands and their tests.

And that’s because 0f190df has the "tranform" typo 😒.

[liZe]

We have to avoid to create a branch on Alembic before merging.

[liZe]

We have to avoid to create a branch on Alembic before merging.

Migrations are only for ping and activation, not for metrics. Everything is OK, this PR can be merged and deployed independently from the other ones.

[adarnimrod]

@liZe I deployed this to the test environment and the migrations failed.

[INFO] alembic.runtime.migration: Running upgrade f2d6141dbece -> 9214d7c1d7d9, Allow only alpha2 country codes in ping_v1 table
[INFO] sqlalchemy.engine.base.Engine: ALTER TABLE ping_v1 DROP CONSTRAINT ck_ping_v1_country_code_2_3_chars

and later on

sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedObject) constraint "ck_ping_v1_country_code_2_3_chars" of relation "ping_v1" does not exist

I checked the database:

\d+ ping_v1;                                                                                                                                                                                                                                        
+---------------------+--------------------------+-------------+-----------+----------------+---------------+
| Column              | Type                     | Modifiers   | Storage   | Stats target   | Description   |
|---------------------+--------------------------+-------------+-----------+----------------+---------------|
| id                  | integer                  |  not null   | plain     | <null>         | <null>        |
| config_id           | integer                  |  not null   | plain     | <null>         | <null>        |
| release             | character varying        |  not null   | extended  | <null>         | <null>        |
| count               | integer                  |  not null   | plain     | <null>         | <null>        |
| country             | character varying(3)     |             | extended  | <null>         | <null>        |
| metrics_enabled     | boolean                  |             | plain     | <null>         | <null>        |
| metrics_environment | character varying        |             | extended  | <null>         | <null>        |
| created_at          | timestamp with time zone |  not null   | plain     | <null>         | <null>        |
+---------------------+--------------------------+-------------+-----------+----------------+---------------+
Indexes:
    "pk_ping_v1" PRIMARY KEY, btree (id)
    "ix_ping_v1_config_id" btree (config_id)
    "ix_ping_v1_created_at" btree (created_at)
Check constraints:
    "ck_ping_v1_ck_ping_v1_country_code_2_3_chars" CHECK (char_length(country::text) = ANY (ARRAY[2, 3]))
    "ck_ping_v1_count_positive" CHECK (count >= 0)
Foreign-key constraints:
    "fk_ping_v1_config_id_ping_configuration_v1" FOREIGN KEY (config_id) REFERENCES ping_configuration_v1(id)
Has OIDs: no

Same for the other environments. I checked the f2d6141dbece migration and the names are the same, so I'm not sure what happened there.

[liZe]

Same for the other environments. I checked the f2d6141dbece migration and the names are the same, so I'm not sure what happened there.

The problem is that naming conventions are applied with op.create_check_constraint, but not with op.drop_constraint. We have to use op.f() to avoid this when using op.create_check_constraint. Alembic usually does this when it automatically generates constraints. But here, automatic generation didn’t work because the constraint was defined in __table_args__ and not in columns (it’s not a standard constraint).

So, I had to generate the migration script myself, and I forgot op.f(). I’m sorry.

Now that f2d6141dbece has been deployed, we have to use the ck_ping_v1_ck_ping_v1_country_code_2_3_chars name (same for activation). I’ve fixed the drop_constraint calls in update. I’ve also added op.f() for the new constraint.

@adarnimrod It’s fixed. I’ve tried to be careful, but checking 71a2069 twice could be useful.