)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"2de8dff931a4f3524234f0253fa29de24eb9b690","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Fix service creation race condition."},{"line_number":10,"context_line":"Return service object instead of creation new instance, if"},{"line_number":11,"context_line":"service with the same host and binary has been already created."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #1634955"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ia5daac1e284bc035a42e971936bfccfeb6fe979d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa6399be_c53bd9cf","line":12,"updated":"2016-10-20 12:10:21.000000000","message":"Silly question, can you explain why the hosts can\u0027t be duplicated? If I have 2 volume services running in the same host, wouldn\u0027t be needed 2 entries?","commit_id":"000a3c8dc52b5b30ae9044114a34e4a68412affa"},{"author":{"_account_id":23602,"name":"Michael Dovgal","email":"dovgalmichael@gmail.com","username":"mdovgal"},"change_message_id":"021bebf404b2d51ba20dc6082831aa67d21cfbd4","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Fix service creation race condition."},{"line_number":10,"context_line":"Return service object instead of creation new instance, if"},{"line_number":11,"context_line":"service with the same host and binary has been already created."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #1634955"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ia5daac1e284bc035a42e971936bfccfeb6fe979d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa6399be_bb37e54b","line":12,"in_reply_to":"fa6399be_c53bd9cf","updated":"2016-10-20 13:54:20.000000000","message":"It\u0027s necessary for cinder-volume service.\nTwo different cinder-volume services cannot manage one backend.\nhttp://paste.openstack.org/show/586561/","commit_id":"000a3c8dc52b5b30ae9044114a34e4a68412affa"}],"cinder/db/sqlalchemy/migrate_repo/versions/085_add_service_host_binary_unique_constraint.py":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"2de8dff931a4f3524234f0253fa29de24eb9b690","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from sqlalchemy import MetaData, Table"},{"line_number":17,"context_line":"from migrate.changeset.constraint import UniqueConstraint"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"def upgrade(migrate_engine):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa6399be_aa26b4cb","line":17,"range":{"start_line":16,"start_character":0,"end_line":17,"end_character":57},"updated":"2016-10-20 12:10:21.000000000","message":"You should not import Objects, only modules.\nhttp://docs.openstack.org/developer/hacking/","commit_id":"000a3c8dc52b5b30ae9044114a34e4a68412affa"},{"author":{"_account_id":23602,"name":"Michael Dovgal","email":"dovgalmichael@gmail.com","username":"mdovgal"},"change_message_id":"021bebf404b2d51ba20dc6082831aa67d21cfbd4","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from sqlalchemy import MetaData, Table"},{"line_number":17,"context_line":"from migrate.changeset.constraint import UniqueConstraint"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"def upgrade(migrate_engine):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa6399be_bb73687f","line":17,"range":{"start_line":16,"start_character":0,"end_line":17,"end_character":57},"in_reply_to":"fa6399be_aa26b4cb","updated":"2016-10-20 13:54:20.000000000","message":"Thank you for review. \nAt the same place there are some exceptions and both of my imports are there.\n\nDo not import objects, only modules (*)\n.......\n(*) exceptions are:\n\nimports from migrate package\nimports from sqlalchemy package","commit_id":"000a3c8dc52b5b30ae9044114a34e4a68412affa"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"09cb4d8706bdf27407f91d67b11f0a1bdc91ec67","unresolved":false,"context_lines":[{"line_number":22,"context_line":"    meta.bind \u003d migrate_engine"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    services \u003d Table(\u0027services\u0027, meta, autoload\u003dTrue)"},{"line_number":25,"context_line":"    cons \u003d UniqueConstraint(\u0027host\u0027, \u0027binary\u0027, table\u003dservices)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    cons.create()"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa6399be_7ee9c9b6","line":25,"range":{"start_line":25,"start_character":0,"end_line":25,"end_character":61},"updated":"2016-10-20 08:26:17.000000000","message":"Are we able to name that constraint and check if it exists before applying? We should aim our DB migrations to be idempotent.","commit_id":"000a3c8dc52b5b30ae9044114a34e4a68412affa"},{"author":{"_account_id":23602,"name":"Michael Dovgal","email":"dovgalmichael@gmail.com","username":"mdovgal"},"change_message_id":"ccc09d7bb04a2e59b7ab9aa9c145d637f76ba77f","unresolved":false,"context_lines":[{"line_number":22,"context_line":"    meta.bind \u003d migrate_engine"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    services \u003d Table(\u0027services\u0027, meta, autoload\u003dTrue)"},{"line_number":25,"context_line":"    cons \u003d UniqueConstraint(\u0027host\u0027, \u0027binary\u0027, table\u003dservices)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    cons.create()"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa6399be_b17d4f1e","line":25,"range":{"start_line":25,"start_character":0,"end_line":25,"end_character":61},"in_reply_to":"fa6399be_7ee9c9b6","updated":"2016-10-20 10:21:16.000000000","message":"Hi, Michal. Thank you for your comment.\nYes, we can add name for constraint and check if it\u0027s already exists to prevent DBError that can be during adding the same constraint for the second time.\n\nHave I got your comment right?","commit_id":"000a3c8dc52b5b30ae9044114a34e4a68412affa"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"cf07e509c7e61db935561ac775aa2d7a17635169","unresolved":false,"context_lines":[{"line_number":22,"context_line":"    meta.bind \u003d migrate_engine"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    services \u003d Table(\u0027services\u0027, meta, autoload\u003dTrue)"},{"line_number":25,"context_line":"    cons \u003d UniqueConstraint(\u0027host\u0027, \u0027binary\u0027, table\u003dservices)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    cons.create()"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa6399be_54a141f7","line":25,"range":{"start_line":25,"start_character":0,"end_line":25,"end_character":61},"in_reply_to":"fa6399be_b17d4f1e","updated":"2016-10-20 10:38:19.000000000","message":"Exactly! :) This is required in case we wanted to backport the bugfix into Newton, so general rule of thumb is that migrations should be idempotent.","commit_id":"000a3c8dc52b5b30ae9044114a34e4a68412affa"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"e3ed66a666bd9b04f52e078e3e676b0855a7f589","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    services \u003d Table(\u0027services\u0027, meta, autoload\u003dTrue)"},{"line_number":28,"context_line":"    # drop all duplicate services, that were already added"},{"line_number":29,"context_line":"    # this line will prevent DBDuplicateEntry error"},{"line_number":30,"context_line":"    utils.drop_old_duplicate_entries_from_table(migrate_engine,"},{"line_number":31,"context_line":"                                                services,"},{"line_number":32,"context_line":"                                                False,"},{"line_number":33,"context_line":"                                                *[\u0027host\u0027, \u0027binary\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_48271212","line":30,"updated":"2016-10-31 10:19:42.000000000","message":"It should be done only if we\u0027ll create a constraint. Please, see my comments below","commit_id":"26264f8ddf0ac00d8c5cce8dfd54462114ce4753"},{"author":{"_account_id":23602,"name":"Michael Dovgal","email":"dovgalmichael@gmail.com","username":"mdovgal"},"change_message_id":"c0a3b52006e513a73e07a350f3c3b08ddfb792fd","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    services \u003d Table(\u0027services\u0027, meta, autoload\u003dTrue)"},{"line_number":28,"context_line":"    # drop all duplicate services, that were already added"},{"line_number":29,"context_line":"    # this line will prevent DBDuplicateEntry error"},{"line_number":30,"context_line":"    utils.drop_old_duplicate_entries_from_table(migrate_engine,"},{"line_number":31,"context_line":"                                                services,"},{"line_number":32,"context_line":"                                                False,"},{"line_number":33,"context_line":"                                                *[\u0027host\u0027, \u0027binary\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_5f3183eb","line":30,"in_reply_to":"ba5da102_48271212","updated":"2016-10-31 12:24:39.000000000","message":"Okey, you are right. I will move it to creating constraint code section.","commit_id":"26264f8ddf0ac00d8c5cce8dfd54462114ce4753"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"e3ed66a666bd9b04f52e078e3e676b0855a7f589","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        if serv_cons[\u0027column_names\u0027] \u003d\u003d [\u0027host\u0027, \u0027binary\u0027]:"},{"line_number":40,"context_line":"            break"},{"line_number":41,"context_line":"        elif serv_cons[\u0027name\u0027] \u003d\u003d \u0027host_binary\u0027:"},{"line_number":42,"context_line":"            cons \u003d UniqueConstraint(table\u003dservices,"},{"line_number":43,"context_line":"                                    name\u003d\u0027host_binary\u0027)"},{"line_number":44,"context_line":"            cons.drop()"},{"line_number":45,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_e815469b","line":42,"updated":"2016-10-31 10:19:42.000000000","message":"Why do we need to drop constraint? IMO, if it\u0027s already added we should just skip a migration logic here","commit_id":"26264f8ddf0ac00d8c5cce8dfd54462114ce4753"},{"author":{"_account_id":23602,"name":"Michael Dovgal","email":"dovgalmichael@gmail.com","username":"mdovgal"},"change_message_id":"c0a3b52006e513a73e07a350f3c3b08ddfb792fd","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        if serv_cons[\u0027column_names\u0027] \u003d\u003d [\u0027host\u0027, \u0027binary\u0027]:"},{"line_number":40,"context_line":"            break"},{"line_number":41,"context_line":"        elif serv_cons[\u0027name\u0027] \u003d\u003d \u0027host_binary\u0027:"},{"line_number":42,"context_line":"            cons \u003d UniqueConstraint(table\u003dservices,"},{"line_number":43,"context_line":"                                    name\u003d\u0027host_binary\u0027)"},{"line_number":44,"context_line":"            cons.drop()"},{"line_number":45,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_1f654be0","line":42,"in_reply_to":"ba5da102_e815469b","updated":"2016-10-31 12:24:39.000000000","message":"Thank you for your comment.\nI will remake this part of code according to your comments","commit_id":"26264f8ddf0ac00d8c5cce8dfd54462114ce4753"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"e3ed66a666bd9b04f52e078e3e676b0855a7f589","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        try:"},{"line_number":48,"context_line":"            cons \u003d UniqueConstraint(\u0027host\u0027, \u0027binary\u0027, table\u003dservices,"},{"line_number":49,"context_line":"                                    name\u003d\u0027host_binary\u0027)"},{"line_number":50,"context_line":"            cons.create()"},{"line_number":51,"context_line":"        except Exception:"},{"line_number":52,"context_line":"            # NOTE (mdovgal): need to clarify exception type"},{"line_number":53,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_884f6abc","line":50,"updated":"2016-10-31 10:19:42.000000000","message":"Should we raise exception here?","commit_id":"26264f8ddf0ac00d8c5cce8dfd54462114ce4753"}],"cinder/objects/service.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"e3ed66a666bd9b04f52e078e3e676b0855a7f589","unresolved":false,"context_lines":[{"line_number":145,"context_line":"                action\u003d\u0027create\u0027, reason\u003d_(\u0027cluster assigned\u0027))"},{"line_number":146,"context_line":"        try:"},{"line_number":147,"context_line":"            db_service \u003d db.service_create(self._context, updates)"},{"line_number":148,"context_line":"        except oslo_db_exception.DBDuplicateEntry:"},{"line_number":149,"context_line":"            raise exception.ServiceExists(host\u003dupdates[\u0027host\u0027],"},{"line_number":150,"context_line":"                                          binary\u003dupdates[\u0027binary\u0027])"},{"line_number":151,"context_line":"        self._from_db_object(self._context, self, db_service)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_08b7da86","line":148,"updated":"2016-10-31 10:19:42.000000000","message":"Please, handle all DB exception in cinder.db.sqlalchemy.api module","commit_id":"26264f8ddf0ac00d8c5cce8dfd54462114ce4753"},{"author":{"_account_id":23602,"name":"Michael Dovgal","email":"dovgalmichael@gmail.com","username":"mdovgal"},"change_message_id":"c0a3b52006e513a73e07a350f3c3b08ddfb792fd","unresolved":false,"context_lines":[{"line_number":145,"context_line":"                action\u003d\u0027create\u0027, reason\u003d_(\u0027cluster assigned\u0027))"},{"line_number":146,"context_line":"        try:"},{"line_number":147,"context_line":"            db_service \u003d db.service_create(self._context, updates)"},{"line_number":148,"context_line":"        except oslo_db_exception.DBDuplicateEntry:"},{"line_number":149,"context_line":"            raise exception.ServiceExists(host\u003dupdates[\u0027host\u0027],"},{"line_number":150,"context_line":"                                          binary\u003dupdates[\u0027binary\u0027])"},{"line_number":151,"context_line":"        self._from_db_object(self._context, self, db_service)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_5f8d037a","line":148,"in_reply_to":"ba5da102_08b7da86","updated":"2016-10-31 12:24:39.000000000","message":"Thank you. I will fix it in the next patch set","commit_id":"26264f8ddf0ac00d8c5cce8dfd54462114ce4753"}]}