)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"460f591f37337280e9b75c3990eca21ffd3a5dec","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Disable utf-8 sanity check by default"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The UTF-8 sanity check is run both before and after migrations. There"},{"line_number":10,"context_line":"has been at least 4 cases now where this sanity check has not caught"},{"line_number":11,"context_line":"a change in the gate but has served to wedge deployers of Keystone."},{"line_number":12,"context_line":"This is because the check should be checking that migrations ensure"},{"line_number":13,"context_line":"utf-8 instead of checking the live DB after and before the migrations."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba3cc151_aa745767","line":10,"updated":"2015-07-09 14:12:56.000000000","message":"Could you please point me to these cases? I must have missed those :(","commit_id":"c9a4ec583a686cce588eedeca216628f51e40de9"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"ba5acc6e7c6eac7e250e7db28cb7640b51aec9bc","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Disable utf-8 sanity check by default"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The UTF-8 sanity check is run both before and after migrations. There"},{"line_number":10,"context_line":"has been at least 4 cases now where this sanity check has not caught"},{"line_number":11,"context_line":"a change in the gate but has served to wedge deployers of Keystone."},{"line_number":12,"context_line":"This is because the check should be checking that migrations ensure"},{"line_number":13,"context_line":"utf-8 instead of checking the live DB after and before the migrations."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a50d1a3_4d6b101f","line":10,"in_reply_to":"ba3cc151_aa745767","updated":"2015-07-20 20:11:45.000000000","message":"I\u0027ll dig them up, however, we will need to go a few releases back, and I can point you at the fixes in our code.","commit_id":"c9a4ec583a686cce588eedeca216628f51e40de9"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"460f591f37337280e9b75c3990eca21ffd3a5dec","unresolved":false,"context_lines":[{"line_number":12,"context_line":"This is because the check should be checking that migrations ensure"},{"line_number":13,"context_line":"utf-8 instead of checking the live DB after and before the migrations."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"For now this just makes the default behavior to not perform the sanity"},{"line_number":16,"context_line":"check (in case projects want to maintain this check). Longer term the"},{"line_number":17,"context_line":"models and migrations should be verified to contain the explciit"},{"line_number":18,"context_line":"charset \u003d utf-8; the change to validate migrations in this manner"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba3cc151_d865f1e0","line":15,"updated":"2015-07-09 14:12:56.000000000","message":"I must disagree with you here.\n\nIt\u0027s simply not enough to make sure our *new* migrations produce utf-8 tables. Before applying any new migrations, we *must* ensure all existing ones use utf-8 charset. Otherwise, we are going to fail right in the middle of a migration script if e.g. it creates adds a new FK from a utf-8 table referencing a latin-1 one:\n\n  import sqlalchemy as sa\n\n\n  meta \u003d sa.MetaData()\n\n\n  parent \u003d sa.Table(\n      \u0027parent\u0027, meta,\n      sa.Column(\u0027a\u0027, sa.Integer, primary_key\u003dTrue),\n      sa.Column(\u0027b\u0027, sa.String(64), index\u003dTrue),\n      mysql_engine\u003d\u0027InnoDB\u0027,\n      mysql_charset\u003d\u0027latin1\u0027\n  )\n\n\n  child \u003d sa.Table(\n      \u0027child\u0027, meta,\n      sa.Column(\u0027a\u0027, sa.Integer, primary_key\u003dTrue),\n      sa.Column(\u0027b\u0027, sa.String(64), sa.ForeignKey(\u0027parent.b\u0027)),\n      mysql_engine\u003d\u0027InnoDB\u0027,\n      mysql_charset\u003d\u0027utf8\u0027\n  )\n\n\n  eng \u003d sa.create_engine(\u0027mysql://root:devel@localhost/devel\u0027,echo\u003dTrue)\n  try:\n      meta.create_all(eng)\n  finally:\n      meta.drop_all(eng, checkfirst\u003dTrue)\n\n\n^ this fails with:\n\nsqlalchemy.exc.OperationalError: (OperationalError) (1005, \"Can\u0027t create table \u0027devel.child\u0027 (errno: 150)\") \u0027\\nCREATE TABLE child (\\n\\ta INTEGER NOT NULL AUTO_INCREMENT, \\n\\tb VARCHAR(64), \\n\\tPRIMARY KEY (a), \\n\\tFOREIGN KEY(b) REFERENCES parent (b)\\n)ENGINE\u003dInnoDB CHARSET\u003dutf8\\n\\n\u0027 ()\n\nwhile making charsets consistent for both tables fixes the problem.\n\n\nSo, IMO, sanity_check\u003dTrue is a better default value. And you can always override it downstream, if you really need to...","commit_id":"c9a4ec583a686cce588eedeca216628f51e40de9"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"4e1644733e9f02f0ccf739d815b1147460a4cc00","unresolved":false,"context_lines":[{"line_number":12,"context_line":"This is because the check should be checking that migrations ensure"},{"line_number":13,"context_line":"utf-8 instead of checking the live DB after and before the migrations."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"For now this just makes the default behavior to not perform the sanity"},{"line_number":16,"context_line":"check (in case projects want to maintain this check). Longer term the"},{"line_number":17,"context_line":"models and migrations should be verified to contain the explciit"},{"line_number":18,"context_line":"charset \u003d utf-8; the change to validate migrations in this manner"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a50d1a3_fa8b971c","line":15,"in_reply_to":"3a50d1a3_4d24700c","updated":"2015-07-21 08:43:13.000000000","message":"It\u0027s not clear to me, how we didn\u0027t catch it in the gate :( \n\nIt\u0027s like if those deployers have changed collation of tables after applying of migration scripts or they simply upgraded their schema from previous releases multiple times, so that tables had been created before we actually added the sanity check.\n\nEither way, it\u0027s still better to fail early, than to fail in the middle of applying of a migration script. So I\u0027d rather we keep the sanity check and provide deployers with clear instructions on how to fix collation of the tables, if they somehow manage to run into this problem.","commit_id":"c9a4ec583a686cce588eedeca216628f51e40de9"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"ba5acc6e7c6eac7e250e7db28cb7640b51aec9bc","unresolved":false,"context_lines":[{"line_number":12,"context_line":"This is because the check should be checking that migrations ensure"},{"line_number":13,"context_line":"utf-8 instead of checking the live DB after and before the migrations."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"For now this just makes the default behavior to not perform the sanity"},{"line_number":16,"context_line":"check (in case projects want to maintain this check). Longer term the"},{"line_number":17,"context_line":"models and migrations should be verified to contain the explciit"},{"line_number":18,"context_line":"charset \u003d utf-8; the change to validate migrations in this manner"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a50d1a3_4d24700c","line":15,"in_reply_to":"ba3cc151_21c59a09","updated":"2015-07-20 20:11:45.000000000","message":"The issue is we don\u0027t catch the problem in gate. This is only biting our deployers. We need another mechanism to ensure the tables are always explicitly set UTF-8. This is beyond insufficient and requires custom 1-off exception code to unwedge deployers.\n\nI\u0027m not sure what the best approach for this is - but checking the tables before/after each migration is incorrect/insufficient.","commit_id":"c9a4ec583a686cce588eedeca216628f51e40de9"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"381b6c402bebcf2cffd988b10b7b5b17574804ba","unresolved":false,"context_lines":[{"line_number":12,"context_line":"This is because the check should be checking that migrations ensure"},{"line_number":13,"context_line":"utf-8 instead of checking the live DB after and before the migrations."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"For now this just makes the default behavior to not perform the sanity"},{"line_number":16,"context_line":"check (in case projects want to maintain this check). Longer term the"},{"line_number":17,"context_line":"models and migrations should be verified to contain the explciit"},{"line_number":18,"context_line":"charset \u003d utf-8; the change to validate migrations in this manner"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba3cc151_21c59a09","line":15,"in_reply_to":"ba3cc151_d865f1e0","updated":"2015-07-10 11:10:48.000000000","message":"...not to mention that in MySQL such a failure would leave the DB schema in inconsistent state (somewhere in between of migration N and N+1) due to lack of transactional DDL support.\n\nSo I\u0027d much rather we failed early during the sanity check.","commit_id":"c9a4ec583a686cce588eedeca216628f51e40de9"}]}