)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"ef0772f61b0d1905f5c78c17a2211b39889d740c","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix issue of password created_at being left as nullable"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Migration 105 left the password created_at column as nullable"},{"line_number":10,"context_line":"which may result in rows created during a migration not having"},{"line_number":11,"context_line":"a date inserted."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Using a combination of triggers and migration scripts in the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1ac06dbe_821a0f16","line":10,"range":{"start_line":10,"start_character":40,"end_line":10,"end_character":51},"updated":"2016-08-19 12:42:53.000000000","message":"an upgrade","commit_id":"387eeb6b881b931298098cdf07e22a245d3a7790"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"7cc6d22e0c9c0113f644b7f4cf32330d083ed16a","unresolved":false,"context_lines":[{"line_number":11,"context_line":"a date inserted."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Using a combination of triggers and migration scripts in the"},{"line_number":14,"context_line":"expand, data migration and contract phases we repair this."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: 1596500"},{"line_number":17,"context_line":"Change-Id: I0f3f26fdfd32de3efe49e79eb5d372932bf06f71"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1ac06dbe_0dc6e247","line":14,"updated":"2016-08-22 16:43:16.000000000","message":"will repair?","commit_id":"590098e507c141c6a1959d208c26812d7edd20d1"}],"keystone/common/sql/contract_repo/versions/002_contract_created_at_non_nullable.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"2076c5113d512dbeeb8248036d1d09d28a810476","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    password_table.c.created_at.alter(nullable\u003dFalse)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    if migrate_engine.name \u003d\u003d \u0027postgresql\u0027:"},{"line_number":30,"context_line":"        stmnt \u003d \u0027DROP TRIGGER IF EXISTS password_created_at ON password;\u0027"},{"line_number":31,"context_line":"    else:"},{"line_number":32,"context_line":"        stmnt \u003d \u0027DROP TRIGGER IF EXISTS password_created_at;\u0027"},{"line_number":33,"context_line":"    migrate_engine.execute(stmnt)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_4703f276","line":30,"updated":"2016-08-24 19:48:59.000000000","message":"This breaks for me in my review because I am not doing `IF EXISTS`\n\nThis is the traceback I get when I run test_walk_versions:\n\nhttp://cdn.pasteraw.com/d085d2zfwthvky30jgx232tdi2pmi8z","commit_id":"e22f54c8a22569b43549e101b49998de20d56c58"},{"author":{"_account_id":17860,"name":"Samuel de Medeiros Queiroz","email":"samueldmq@gmail.com","username":"samueldmq"},"change_message_id":"7cc6f2d7772cee173a054754e39f34cf4eddb098","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    password_table.c.created_at.alter(nullable\u003dFalse)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    if migrate_engine.name \u003d\u003d \u0027postgresql\u0027:"},{"line_number":30,"context_line":"        stmnt \u003d \u0027DROP TRIGGER IF EXISTS password_created_at ON password;\u0027"},{"line_number":31,"context_line":"    else:"},{"line_number":32,"context_line":"        stmnt \u003d \u0027DROP TRIGGER IF EXISTS password_created_at;\u0027"},{"line_number":33,"context_line":"    migrate_engine.execute(stmnt)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_3300a2aa","line":30,"in_reply_to":"1ac06dbe_4703f276","updated":"2016-08-25 03:08:05.000000000","message":"Any reason why \u0027IF EXISTS\u0027 should be removed ?","commit_id":"e22f54c8a22569b43549e101b49998de20d56c58"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"1bf16ebce83ad55d7f77e9683c217fdbb6ce7655","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    password_table.c.created_at.alter(nullable\u003dFalse)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    if migrate_engine.name \u003d\u003d \u0027postgresql\u0027:"},{"line_number":30,"context_line":"        stmnt \u003d \u0027DROP TRIGGER IF EXISTS password_created_at ON password;\u0027"},{"line_number":31,"context_line":"    else:"},{"line_number":32,"context_line":"        stmnt \u003d \u0027DROP TRIGGER IF EXISTS password_created_at;\u0027"},{"line_number":33,"context_line":"    migrate_engine.execute(stmnt)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_56b55ece","line":30,"in_reply_to":"1ac06dbe_4703f276","updated":"2016-08-24 21:10:38.000000000","message":"I\u0027m going to test this locally and see if it\u0027s consistent with what I\u0027m seeing.","commit_id":"e22f54c8a22569b43549e101b49998de20d56c58"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"3436b4e758647613d543cffe0c191a634e996a13","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    password_table.c.created_at.alter(nullable\u003dFalse)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    if migrate_engine.name \u003d\u003d \u0027postgresql\u0027:"},{"line_number":30,"context_line":"        stmnt \u003d \u0027DROP TRIGGER IF EXISTS password_created_at ON password;\u0027"},{"line_number":31,"context_line":"    else:"},{"line_number":32,"context_line":"        stmnt \u003d \u0027DROP TRIGGER IF EXISTS password_created_at;\u0027"},{"line_number":33,"context_line":"    migrate_engine.execute(stmnt)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_61ed368e","line":30,"in_reply_to":"1ac06dbe_56b55ece","updated":"2016-08-24 21:21:32.000000000","message":"So when I remove the IF EXISTS conditional locally (with http://cdn.pasteraw.com/rdkduii03944sy4b6pezz5p1glj20ts) I get two test failures: \n\nhttp://cdn.pasteraw.com/42zam2htmobhucnhuhu0gxqf04onywi\n\nhttp://cdn.pasteraw.com/a7hv8t4reqpfmfuy0r7oakqg03worxc","commit_id":"e22f54c8a22569b43549e101b49998de20d56c58"}],"keystone/common/sql/expand_repo/versions/002_mysql_upgrade.sql":[{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"3647ccbf0a1cf52eca6f8d5335b53e6e575a17c4","unresolved":false,"context_lines":[{"line_number":1,"context_line":"DROP TRIGGER IF EXISTS password_created_at;"},{"line_number":2,"context_line":"CREATE TRIGGER password_created_at BEFORE INSERT ON password"},{"line_number":3,"context_line":"FOR EACH ROW"},{"line_number":4,"context_line":"BEGIN"}],"source_content_type":"text/x-sql","patch_set":3,"id":"1ac06dbe_aac0547d","line":1,"range":{"start_line":1,"start_character":0,"end_line":1,"end_character":43},"updated":"2016-08-22 15:58:26.000000000","message":"I\u0027m torn as to whether you should try to drop it first. On one level, there should be not trigger yet...on the other, concerned that any problems in a an grad might leave them in place and hence cause this migrate to fail.  Another black mark against dropping might be that this wold complicate automatic checking for banned operations (since in theory dropping should only happen in the contract phase)","commit_id":"590098e507c141c6a1959d208c26812d7edd20d1"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"87d9f73a794c310de53fea178a3e2ce56c741b31","unresolved":false,"context_lines":[{"line_number":1,"context_line":"DROP TRIGGER IF EXISTS password_created_at;"},{"line_number":2,"context_line":"CREATE TRIGGER password_created_at BEFORE INSERT ON password"},{"line_number":3,"context_line":"FOR EACH ROW"},{"line_number":4,"context_line":"BEGIN"}],"source_content_type":"text/x-sql","patch_set":3,"id":"1ac06dbe_1cc1e954","line":1,"updated":"2016-08-22 18:43:47.000000000","message":"Side note - is it possible to have a trigger created using this file and have a migration named 002_more_stuff executed at the same time?","commit_id":"590098e507c141c6a1959d208c26812d7edd20d1"},{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"efc708e352fdcf6b6c1f78354b3491370a56d357","unresolved":false,"context_lines":[{"line_number":1,"context_line":"DROP TRIGGER IF EXISTS password_created_at;"},{"line_number":2,"context_line":"CREATE TRIGGER password_created_at BEFORE INSERT ON password"},{"line_number":3,"context_line":"FOR EACH ROW"},{"line_number":4,"context_line":"BEGIN"}],"source_content_type":"text/x-sql","patch_set":3,"id":"1ac06dbe_b1bc1c07","line":1,"in_reply_to":"1ac06dbe_1cc1e954","updated":"2016-08-23 09:28:24.000000000","message":"No, from looking at the code I don\u0027t believe so. If there is a db specific file that is used, if not then the general one is used. It does mean that you could just have a db specific file for, say, postgresql - and have the .py file handle the other databases.","commit_id":"590098e507c141c6a1959d208c26812d7edd20d1"},{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"73bd3e34257b8b4a54770c6a599bf3d6a323f307","unresolved":false,"context_lines":[{"line_number":1,"context_line":"DROP TRIGGER IF EXISTS password_created_at;"},{"line_number":2,"context_line":"CREATE TRIGGER password_created_at BEFORE INSERT ON password"},{"line_number":3,"context_line":"FOR EACH ROW"},{"line_number":4,"context_line":"BEGIN"}],"source_content_type":"text/x-sql","patch_set":3,"id":"1ac06dbe_d67fe012","line":1,"in_reply_to":"1ac06dbe_905730c9","updated":"2016-08-23 19:10:01.000000000","message":"...or you write all your migration in sql....","commit_id":"590098e507c141c6a1959d208c26812d7edd20d1"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"3db72cbc038c42aae43df4b97e0ad6f00421dc4e","unresolved":false,"context_lines":[{"line_number":1,"context_line":"DROP TRIGGER IF EXISTS password_created_at;"},{"line_number":2,"context_line":"CREATE TRIGGER password_created_at BEFORE INSERT ON password"},{"line_number":3,"context_line":"FOR EACH ROW"},{"line_number":4,"context_line":"BEGIN"}],"source_content_type":"text/x-sql","patch_set":3,"id":"1ac06dbe_b92707c9","line":1,"in_reply_to":"1ac06dbe_aac0547d","updated":"2016-08-22 18:36:16.000000000","message":"I would think that we should fail the migration phase if a trigger already exists. In that case - we should probably inform the operator and give them an opportunity to fix whatever the issue is and re-run the phase from a clean state.","commit_id":"590098e507c141c6a1959d208c26812d7edd20d1"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"47ec5b9c80a36f1a8f834931ff25a3898613663c","unresolved":false,"context_lines":[{"line_number":1,"context_line":"DROP TRIGGER IF EXISTS password_created_at;"},{"line_number":2,"context_line":"CREATE TRIGGER password_created_at BEFORE INSERT ON password"},{"line_number":3,"context_line":"FOR EACH ROW"},{"line_number":4,"context_line":"BEGIN"}],"source_content_type":"text/x-sql","patch_set":3,"id":"1ac06dbe_905730c9","line":1,"in_reply_to":"1ac06dbe_b1bc1c07","updated":"2016-08-23 18:20:33.000000000","message":"So that means for migrations that only need to add a trigger you can get by using a .sql file.. otherwise the trigger will have to be created in the migration script by executing sql manually.","commit_id":"590098e507c141c6a1959d208c26812d7edd20d1"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"481ef9a807310a9b28cb200e0041114b022af0eb","unresolved":false,"context_lines":[{"line_number":2,"context_line":"FOR EACH ROW"},{"line_number":3,"context_line":"BEGIN"},{"line_number":4,"context_line":"  IF NEW.created_at IS NULL THEN"},{"line_number":5,"context_line":"    SET NEW.created_at \u003d now();"},{"line_number":6,"context_line":"  END IF;"},{"line_number":7,"context_line":"END;"}],"source_content_type":"text/x-sql","patch_set":6,"id":"fa7ab95a_7a1cc113","line":5,"updated":"2016-08-26 17:21:28.000000000","message":"Why not just put a SQL level DEFAULT on the column ?  that\u0027s the more usual way to handle simple INSERT defaults.","commit_id":"ba721aaba6ad5042a3f4a2fb822c529e1a34ad30"},{"author":{"_account_id":18338,"name":"Ron De Rose","email":"ronald.de.rose@intel.com","username":"derosenet"},"change_message_id":"8115f9cf3d4a0def0032da9470ca82cab726f158","unresolved":false,"context_lines":[{"line_number":2,"context_line":"FOR EACH ROW"},{"line_number":3,"context_line":"BEGIN"},{"line_number":4,"context_line":"  IF NEW.created_at IS NULL THEN"},{"line_number":5,"context_line":"    SET NEW.created_at \u003d now();"},{"line_number":6,"context_line":"  END IF;"},{"line_number":7,"context_line":"END;"}],"source_content_type":"text/x-sql","patch_set":6,"id":"fa7ab95a_88c810aa","line":5,"in_reply_to":"fa7ab95a_04618f39","updated":"2016-08-29 13:26:20.000000000","message":"@Henry based on Michael\u0027s response, you could close this bug by modifying version 105, setting the column default and if sqlite, update the column values like so: \nhttp://paste.openstack.org/show/564574/\n\nWould just need to confirm that the default works for both mysql and postgres.","commit_id":"ba721aaba6ad5042a3f4a2fb822c529e1a34ad30"},{"author":{"_account_id":18338,"name":"Ron De Rose","email":"ronald.de.rose@intel.com","username":"derosenet"},"change_message_id":"1302c1aef48c08742669288f853c049f96838b39","unresolved":false,"context_lines":[{"line_number":2,"context_line":"FOR EACH ROW"},{"line_number":3,"context_line":"BEGIN"},{"line_number":4,"context_line":"  IF NEW.created_at IS NULL THEN"},{"line_number":5,"context_line":"    SET NEW.created_at \u003d now();"},{"line_number":6,"context_line":"  END IF;"},{"line_number":7,"context_line":"END;"}],"source_content_type":"text/x-sql","patch_set":6,"id":"fa7ab95a_a9f3619d","line":5,"in_reply_to":"fa7ab95a_7a1cc113","updated":"2016-08-26 19:01:46.000000000","message":"Agree.  But I tried adding the DEFAULT with sqlalchemy migrate to no avail. Tried several options, including:\n* default\u003ddatetime.datetime.utcnow\n* default\u003dsql.func.now()\n* server_default\u003dsql.func.now()\n* ...\n\nBut it wouldn\u0027t work with all of the supported databases. For example, I\u0027d get it working with mysql, but would not work with sqlite. \n\nAnd I believe server_default only works for table creates; not for table alters.","commit_id":"ba721aaba6ad5042a3f4a2fb822c529e1a34ad30"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"6b272fa243a197c78002bafeb062c0892c93360c","unresolved":false,"context_lines":[{"line_number":2,"context_line":"FOR EACH ROW"},{"line_number":3,"context_line":"BEGIN"},{"line_number":4,"context_line":"  IF NEW.created_at IS NULL THEN"},{"line_number":5,"context_line":"    SET NEW.created_at \u003d now();"},{"line_number":6,"context_line":"  END IF;"},{"line_number":7,"context_line":"END;"}],"source_content_type":"text/x-sql","patch_set":6,"id":"fa7ab95a_e056105b","line":5,"in_reply_to":"fa7ab95a_88c810aa","updated":"2016-08-29 14:15:04.000000000","message":"not just that, but also create a new migration that sets it, in case users have already run migration 105..","commit_id":"ba721aaba6ad5042a3f4a2fb822c529e1a34ad30"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"ecc53b0915a1938ee98c46216bb3c1b8884d8912","unresolved":false,"context_lines":[{"line_number":2,"context_line":"FOR EACH ROW"},{"line_number":3,"context_line":"BEGIN"},{"line_number":4,"context_line":"  IF NEW.created_at IS NULL THEN"},{"line_number":5,"context_line":"    SET NEW.created_at \u003d now();"},{"line_number":6,"context_line":"  END IF;"},{"line_number":7,"context_line":"END;"}],"source_content_type":"text/x-sql","patch_set":6,"id":"fa7ab95a_04618f39","line":5,"in_reply_to":"fa7ab95a_a9f3619d","updated":"2016-08-26 20:28:41.000000000","message":"defaults work with ALTER.  SQLite does not support ALTER, so sqlalchemy-migrate drops the whole table and recreates it.  should still work in the end.   Has to be server_default, however.\n\nAlso, SQLite isn\u0027t used in production.  There\u0027s no need for the default to work on that backend in any case except for the benefit of tests, which IMO should be against MySQL/Postgresql in any case since that\u0027s where this feature needs to be tested.","commit_id":"ba721aaba6ad5042a3f4a2fb822c529e1a34ad30"}]}