)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"69942b4b144fb6c5c883c3190a2595f415922560","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Default security group table"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change prevents the race condition by enforcing a single default "},{"line_number":10,"context_line":"security group via new table defaultsecuritygroups. It has tenant_id "},{"line_number":11,"context_line":"as primary key and security_group_id, which is id of default "},{"line_number":12,"context_line":"security group."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"3a961159_a6f52552","line":9,"updated":"2014-12-23 14:15:07.000000000","message":"nits: some trailing spaces to remove","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"609e5176ccfa90dc4710d9074fc570d084c5547f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Default security group table"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change prevents the race condition by enforcing a single default "},{"line_number":10,"context_line":"security group via new table defaultsecuritygroups. It has tenant_id "},{"line_number":11,"context_line":"as primary key and security_group_id, which is id of default "},{"line_number":12,"context_line":"security group."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"3a961159_f207e315","line":9,"in_reply_to":"3a961159_a6f52552","updated":"2014-12-23 16:02:49.000000000","message":"Thanks, will remove, just I look forward more comments on this change.","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7962,"name":"Ronak Shah","email":"ronak.malav.shah@gmail.com","username":"ronak"},"change_message_id":"afb08b13d35862463d82d1f63e781ee0c0bbe130","unresolved":false,"context_lines":[{"line_number":12,"context_line":"security group."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This idea has come up from discussion in comments to"},{"line_number":15,"context_line":"https://review.openstack.org/135006."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Closes-bug: #1194579"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3a961159_e295a0e4","line":15,"updated":"2015-01-06 07:15:47.000000000","message":"remove the dot at the end of the hyperlink above.","commit_id":"c3f094ead6d12cfa7a11c178ecd0da935f2b0c93"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"4ac2287defe1cdcf730dd33e23b5a1dcfa0e8453","unresolved":false,"context_lines":[{"line_number":12,"context_line":"security group."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This idea has come up from discussion in comments to"},{"line_number":15,"context_line":"https://review.openstack.org/135006."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Closes-bug: #1194579"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3a961159_794637c2","line":15,"in_reply_to":"3a961159_e295a0e4","updated":"2015-01-12 11:00:37.000000000","message":"done","commit_id":"c3f094ead6d12cfa7a11c178ecd0da935f2b0c93"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c1332a23d009dd83a70bc5d43649544c7707bb68","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Default security group table"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change prevents the race condition by enforcing a single default"},{"line_number":10,"context_line":"security group via new table defaultsecuritygroups. It has tenant_id"},{"line_number":11,"context_line":"as primary key and security_group_id, which is id of default"},{"line_number":12,"context_line":"security group."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"1a930d6b_4c02760c","line":10,"updated":"2015-01-22 15:00:08.000000000","message":"Is the name of the table correct? As per migration script, it\u0027s \u0027default_security_group\u0027. BTW why do we even use _ separators? It does not seem to be a common approach for other tables (f.e. see securitygroups table in your migration script).","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"d92f47d25d67170e232d2efc9f7c817906e1942c","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Default security group table"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change prevents the race condition by enforcing a single default"},{"line_number":10,"context_line":"security group via new table defaultsecuritygroups. It has tenant_id"},{"line_number":11,"context_line":"as primary key and security_group_id, which is id of default"},{"line_number":12,"context_line":"security group."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"1a930d6b_2fab5c1e","line":10,"in_reply_to":"1a930d6b_4c02760c","updated":"2015-01-22 15:13:32.000000000","message":"In patch set 5 I\u0027ve got comment from Maru to rename using _. Seems that I forgot to rename it here. I\u0027m not sure that there is any common approach as I during my work on neutron database saw existing both variants . Lasted added tables have _.","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"}],"neutron/db/migration/__init__.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ea6c6574fe2ad03d0556cebd30878e75bf431fd1","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            .subquery())"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    sg \u003d (session.query(SecurityGroup)"},{"line_number":192,"context_line":"          .join(subq, SecurityGroup.c.tenant_id \u003d\u003d subq.c.tenant_id)"},{"line_number":193,"context_line":"          .filter(SecurityGroup.c.name \u003d\u003d \u0027default\u0027)"},{"line_number":194,"context_line":"          .all())"},{"line_number":195,"context_line":"    res \u003d {}"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_1bd61254","line":192,"updated":"2015-01-19 23:53:30.000000000","message":"I\u0027m not sure why a join should be required - what am I missing?","commit_id":"e91fbea5ea29edd58efe632f442bb11508345221"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"9189aeda6cc287a812df419f5996dee35f51fc6b","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            .subquery())"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    sg \u003d (session.query(SecurityGroup)"},{"line_number":192,"context_line":"          .join(subq, SecurityGroup.c.tenant_id \u003d\u003d subq.c.tenant_id)"},{"line_number":193,"context_line":"          .filter(SecurityGroup.c.name \u003d\u003d \u0027default\u0027)"},{"line_number":194,"context_line":"          .all())"},{"line_number":195,"context_line":"    res \u003d {}"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_c4c3c204","line":192,"in_reply_to":"3a961159_1bd61254","updated":"2015-01-20 09:24:27.000000000","message":"Subquery selects all tenant_id\u0027s that have more than one \u0027default\u0027 security group. This query joins result of subquery with securitygroups table to get actual security groups\u0027 ids. This cannot be done without joining table with aggregation results.","commit_id":"e91fbea5ea29edd58efe632f442bb11508345221"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ea6c6574fe2ad03d0556cebd30878e75bf431fd1","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        res.setdefault(s.tenant_id, []).append(s.id)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    if res:"},{"line_number":200,"context_line":"        raise DuplicateSecurityGroupsNamedDefault("},{"line_number":201,"context_line":"            duplications\u003d\u0027; \u0027.join(\u0027tenant %s: %s\u0027 %"},{"line_number":202,"context_line":"                                   (tenant_id, \u0027, \u0027.join(groups))"},{"line_number":203,"context_line":"                                   for tenant_id, groups in res.iteritems()))"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_9b83e254","line":200,"updated":"2015-01-19 23:53:30.000000000","message":"Please ensure that this block is executed in testing.  One possibility would be to target duplicate default groups.  Another would be separating lines 199-203 into a function that would accept a resultset and could be targeted in a unit test, e.g.\n\ndef check_default_groups(result_set):\n  if result_set:\n     raise ...","commit_id":"e91fbea5ea29edd58efe632f442bb11508345221"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"9189aeda6cc287a812df419f5996dee35f51fc6b","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        res.setdefault(s.tenant_id, []).append(s.id)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    if res:"},{"line_number":200,"context_line":"        raise DuplicateSecurityGroupsNamedDefault("},{"line_number":201,"context_line":"            duplications\u003d\u0027; \u0027.join(\u0027tenant %s: %s\u0027 %"},{"line_number":202,"context_line":"                                   (tenant_id, \u0027, \u0027.join(groups))"},{"line_number":203,"context_line":"                                   for tenant_id, groups in res.iteritems()))"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_240e468d","line":200,"in_reply_to":"3a961159_9b83e254","updated":"2015-01-20 09:24:27.000000000","message":"I mark this patch as WIP, as I haven\u0027t created tests for this yet and working on better solution.Thanks for suggestion.","commit_id":"e91fbea5ea29edd58efe632f442bb11508345221"}],"neutron/db/migration/alembic_migrations/versions/14be42f3d0a5_default_sec_group_table.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"28436f22e743f0385f3056d86427bc7c46711af6","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    dialect_name \u003d op.get_bind().engine.dialect.name"},{"line_number":34,"context_line":"    op.create_table(\u0027defaultsecuritygroups\u0027,"},{"line_number":35,"context_line":"                    sa.Column(\u0027tenant_id\u0027, sa.String(length\u003d255),"},{"line_number":36,"context_line":"                              nullable\u003dFalse),"},{"line_number":37,"context_line":"                    sa.Column(\u0027security_group_id\u0027, sa.String(length\u003d36),"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_32a0848f","line":34,"updated":"2014-12-23 21:05:25.000000000","message":"Please use underscores to separate words in table names.","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"48b7d93acdb68cfe89502d01646d0ee5d3968c67","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    dialect_name \u003d op.get_bind().engine.dialect.name"},{"line_number":34,"context_line":"    op.create_table(\u0027defaultsecuritygroups\u0027,"},{"line_number":35,"context_line":"                    sa.Column(\u0027tenant_id\u0027, sa.String(length\u003d255),"},{"line_number":36,"context_line":"                              nullable\u003dFalse),"},{"line_number":37,"context_line":"                    sa.Column(\u0027security_group_id\u0027, sa.String(length\u003d36),"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_767add78","line":34,"in_reply_to":"3a961159_32a0848f","updated":"2014-12-25 12:29:51.000000000","message":"done","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"69942b4b144fb6c5c883c3190a2595f415922560","unresolved":false,"context_lines":[{"line_number":38,"context_line":"                              nullable\u003dFalse),"},{"line_number":39,"context_line":"                    sa.PrimaryKeyConstraint(\u0027tenant_id\u0027),"},{"line_number":40,"context_line":"                    )"},{"line_number":41,"context_line":"    if dialect_name \u003d\u003d \u0027mysql\u0027:"},{"line_number":42,"context_line":"        op.execute(\"UPDATE defaultsecuritygroups AS t1, \""},{"line_number":43,"context_line":"                   \"(SELECT id, tenant_id FROM securitygroups \""},{"line_number":44,"context_line":"                   \"WHERE name\u003d\u0027default\u0027) AS t2 \""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_61da4779","line":41,"updated":"2014-12-23 14:15:07.000000000","message":"Sorry for potentially dumb question, but how will it work with duplicate groups with \u0027default\u0027 name that already exist in db?","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"e65119560f3feb49ffe600d6d3c0763d1f35d302","unresolved":false,"context_lines":[{"line_number":38,"context_line":"                              nullable\u003dFalse),"},{"line_number":39,"context_line":"                    sa.PrimaryKeyConstraint(\u0027tenant_id\u0027),"},{"line_number":40,"context_line":"                    )"},{"line_number":41,"context_line":"    if dialect_name \u003d\u003d \u0027mysql\u0027:"},{"line_number":42,"context_line":"        op.execute(\"UPDATE defaultsecuritygroups AS t1, \""},{"line_number":43,"context_line":"                   \"(SELECT id, tenant_id FROM securitygroups \""},{"line_number":44,"context_line":"                   \"WHERE name\u003d\u0027default\u0027) AS t2 \""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_b231d4a3","line":41,"in_reply_to":"3a961159_17df013b","updated":"2014-12-23 19:36:14.000000000","message":"Yeah, but we know that we have a race condition in the code that could already create two \u0027default\u0027 named groups. So won\u0027t migration code fail in that case?","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"b493307e5d282359c678a7148ca6e432c54d3db1","unresolved":false,"context_lines":[{"line_number":38,"context_line":"                              nullable\u003dFalse),"},{"line_number":39,"context_line":"                    sa.PrimaryKeyConstraint(\u0027tenant_id\u0027),"},{"line_number":40,"context_line":"                    )"},{"line_number":41,"context_line":"    if dialect_name \u003d\u003d \u0027mysql\u0027:"},{"line_number":42,"context_line":"        op.execute(\"UPDATE defaultsecuritygroups AS t1, \""},{"line_number":43,"context_line":"                   \"(SELECT id, tenant_id FROM securitygroups \""},{"line_number":44,"context_line":"                   \"WHERE name\u003d\u0027default\u0027) AS t2 \""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_1198d573","line":41,"in_reply_to":"3a961159_52fa7043","updated":"2014-12-24 12:48:10.000000000","message":"OK, will work on this.","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"b11cacc3ae52164a536bb1823982527fbb1bda60","unresolved":false,"context_lines":[{"line_number":38,"context_line":"                              nullable\u003dFalse),"},{"line_number":39,"context_line":"                    sa.PrimaryKeyConstraint(\u0027tenant_id\u0027),"},{"line_number":40,"context_line":"                    )"},{"line_number":41,"context_line":"    if dialect_name \u003d\u003d \u0027mysql\u0027:"},{"line_number":42,"context_line":"        op.execute(\"UPDATE defaultsecuritygroups AS t1, \""},{"line_number":43,"context_line":"                   \"(SELECT id, tenant_id FROM securitygroups \""},{"line_number":44,"context_line":"                   \"WHERE name\u003d\u0027default\u0027) AS t2 \""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_17df013b","line":41,"in_reply_to":"3a961159_61da4779","updated":"2014-12-23 15:45:37.000000000","message":"We have only one default security group for each tenant and user can\u0027t create security group with name \u0027default\u0027 himself https://github.com/openstack/neutron/blob/master/neutron/extensions/securitygroup.py#L176","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"28436f22e743f0385f3056d86427bc7c46711af6","unresolved":false,"context_lines":[{"line_number":38,"context_line":"                              nullable\u003dFalse),"},{"line_number":39,"context_line":"                    sa.PrimaryKeyConstraint(\u0027tenant_id\u0027),"},{"line_number":40,"context_line":"                    )"},{"line_number":41,"context_line":"    if dialect_name \u003d\u003d \u0027mysql\u0027:"},{"line_number":42,"context_line":"        op.execute(\"UPDATE defaultsecuritygroups AS t1, \""},{"line_number":43,"context_line":"                   \"(SELECT id, tenant_id FROM securitygroups \""},{"line_number":44,"context_line":"                   \"WHERE name\u003d\u0027default\u0027) AS t2 \""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_52fa7043","line":41,"in_reply_to":"3a961159_b231d4a3","updated":"2014-12-23 21:05:25.000000000","message":"This is a fair point.  What do you think of failing the migration if there are duplicates and adding a script that can generate a report of duplicates so that the problem can be manually remedied?","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1cc54f97df35ac9500e2f12ef0ac567d9d31d386","unresolved":false,"context_lines":[{"line_number":49,"context_line":"                   \"(select tenant_id, id \""},{"line_number":50,"context_line":"                   \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"},{"line_number":51,"context_line":"    except exception.DBDuplicateEntry:"},{"line_number":52,"context_line":"        raise ext_sg.DuplicateSecurityGroupsNamedDefault()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def downgrade():"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_d06a95ec","line":52,"updated":"2015-01-12 14:05:42.000000000","message":"So what\u0027s our solution for users with the problem? Should we provide some tool to clear duplicate entries? Or do we think it\u0027s enough to report the general error and make operator search for duplicate objects and clean them up?","commit_id":"0c4c8919bc27ca7fdb0b5efa0ef8c99641cea0ed"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"011acb8ddf1df899b3c88fb438bb134a54ae8a86","unresolved":false,"context_lines":[{"line_number":49,"context_line":"                   \"(select tenant_id, id \""},{"line_number":50,"context_line":"                   \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"},{"line_number":51,"context_line":"    except exception.DBDuplicateEntry:"},{"line_number":52,"context_line":"        raise ext_sg.DuplicateSecurityGroupsNamedDefault()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def downgrade():"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_411a5919","line":52,"in_reply_to":"3a961159_213e2dba","updated":"2015-01-12 14:59:09.000000000","message":"How should be decided what should be clean up? I agreed on adding information about duplication I will think about better way to do it, but I think that users should decide what should be deleted themselves.","commit_id":"0c4c8919bc27ca7fdb0b5efa0ef8c99641cea0ed"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"09de6f0fe686d7ade7d7d3b63edad01c48630ddc","unresolved":false,"context_lines":[{"line_number":49,"context_line":"                   \"(select tenant_id, id \""},{"line_number":50,"context_line":"                   \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"},{"line_number":51,"context_line":"    except exception.DBDuplicateEntry:"},{"line_number":52,"context_line":"        raise ext_sg.DuplicateSecurityGroupsNamedDefault()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def downgrade():"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_213e2dba","line":52,"in_reply_to":"3a961159_2b79ae30","updated":"2015-01-12 14:50:37.000000000","message":"I think Maru suggested before to add a script that would help users identify the duplicate objects. (And probably also clean them up.)","commit_id":"0c4c8919bc27ca7fdb0b5efa0ef8c99641cea0ed"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"48b5f5c9f519650cb8778fece32a9b43e436ddc4","unresolved":false,"context_lines":[{"line_number":49,"context_line":"                   \"(select tenant_id, id \""},{"line_number":50,"context_line":"                   \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"},{"line_number":51,"context_line":"    except exception.DBDuplicateEntry:"},{"line_number":52,"context_line":"        raise ext_sg.DuplicateSecurityGroupsNamedDefault()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def downgrade():"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_a1c4dd0e","line":52,"in_reply_to":"3a961159_411a5919","updated":"2015-01-12 15:06:32.000000000","message":"At the very least, we could inform about which tenant failed migration. Can we include the data into the exception that we raise?","commit_id":"0c4c8919bc27ca7fdb0b5efa0ef8c99641cea0ed"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"048c65372cdd32b180ee5aa5d55f5104575ef458","unresolved":false,"context_lines":[{"line_number":49,"context_line":"                   \"(select tenant_id, id \""},{"line_number":50,"context_line":"                   \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"},{"line_number":51,"context_line":"    except exception.DBDuplicateEntry:"},{"line_number":52,"context_line":"        raise ext_sg.DuplicateSecurityGroupsNamedDefault()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def downgrade():"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_49a2784c","line":52,"in_reply_to":"3a961159_a1c4dd0e","updated":"2015-01-12 19:25:35.000000000","message":"I agree I think it would if we could return the security groups where there are duplicates so an operator can more easily resolve the problem and an error message describing the issue.","commit_id":"0c4c8919bc27ca7fdb0b5efa0ef8c99641cea0ed"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"452324ac14e89a1b9505c4f7e820bf28cefa2b01","unresolved":false,"context_lines":[{"line_number":49,"context_line":"                   \"(select tenant_id, id \""},{"line_number":50,"context_line":"                   \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"},{"line_number":51,"context_line":"    except exception.DBDuplicateEntry:"},{"line_number":52,"context_line":"        raise ext_sg.DuplicateSecurityGroupsNamedDefault()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def downgrade():"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_2b79ae30","line":52,"in_reply_to":"3a961159_d06a95ec","updated":"2015-01-12 14:27:15.000000000","message":"In fact there should not be more than one default security in one tenant, so if it appears it looks more like a mistake. In patch set 5 Maru Newby suggested this approach and I agreed that it is suitable. The only way to fix this automatically that I see is to rename duplicated security groups, which you don\u0027t like in https://review.openstack.org/135006. If something else is in your mind I will be happy to discuss.","commit_id":"0c4c8919bc27ca7fdb0b5efa0ef8c99641cea0ed"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"96a4d892b7f4e5a0850bf20459db1cd2c343e490","unresolved":false,"context_lines":[{"line_number":58,"context_line":"            sg_model.SecurityGroup.name \u003d\u003d \u0027default\u0027,"},{"line_number":59,"context_line":"            sg_model.SecurityGroup.tenant_id \u003d\u003d ex.value).all()"},{"line_number":60,"context_line":"        raise ext_sg.DuplicateSecurityGroupsNamedDefault("},{"line_number":61,"context_line":"            tenant_id\u003dsg[0].tenant_id, sg_ids\u003d[s.id for s in sg])"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"def downgrade():"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_1f405734","line":61,"updated":"2015-01-14 14:06:32.000000000","message":"So we still report just the first bad tenant. Can we somehow report all of them? If not here in migration code, then with a tool maybe?","commit_id":"1bbcd8695093a5482d0d97a746d447d56ba2e2eb"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"91bfecbfbea406c4773a16ee3ed941b090b828dd","unresolved":false,"context_lines":[{"line_number":58,"context_line":"            sg_model.SecurityGroup.name \u003d\u003d \u0027default\u0027,"},{"line_number":59,"context_line":"            sg_model.SecurityGroup.tenant_id \u003d\u003d ex.value).all()"},{"line_number":60,"context_line":"        raise ext_sg.DuplicateSecurityGroupsNamedDefault("},{"line_number":61,"context_line":"            tenant_id\u003dsg[0].tenant_id, sg_ids\u003d[s.id for s in sg])"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"def downgrade():"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_43ae85e8","line":61,"in_reply_to":"3a961159_1f405734","updated":"2015-01-14 16:48:09.000000000","message":"add in next patch set","commit_id":"1bbcd8695093a5482d0d97a746d447d56ba2e2eb"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f1bb14f06b10d38e57091e39f4f6a0de8d1bf5b3","unresolved":false,"context_lines":[{"line_number":54,"context_line":"                   \"(tenant_id, security_group_id) \""},{"line_number":55,"context_line":"                   \"(select tenant_id, id \""},{"line_number":56,"context_line":"                   \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"},{"line_number":57,"context_line":"    except exception.DBDuplicateEntry:"},{"line_number":58,"context_line":"        # Models can change in time, but migration should rely only on exact"},{"line_number":59,"context_line":"        # model state at the current moment, so a separate model is created"},{"line_number":60,"context_line":"        # here."}],"source_content_type":"text/x-python","patch_set":15,"id":"3a961159_e800fa36","line":57,"updated":"2015-01-15 20:40:59.000000000","message":"This failure mode looks complicated enough to require a test addition.  Hopefully your work on migration testing can be a good starting point?","commit_id":"a8f0c20e1c895ae2a45aeedb1e4cfee5431bce45"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"312092f34d18aa242fd5af7252d1603c6faa9d58","unresolved":false,"context_lines":[{"line_number":54,"context_line":"                   \"(tenant_id, security_group_id) \""},{"line_number":55,"context_line":"                   \"(select tenant_id, id \""},{"line_number":56,"context_line":"                   \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"},{"line_number":57,"context_line":"    except exception.DBDuplicateEntry:"},{"line_number":58,"context_line":"        # Models can change in time, but migration should rely only on exact"},{"line_number":59,"context_line":"        # model state at the current moment, so a separate model is created"},{"line_number":60,"context_line":"        # here."}],"source_content_type":"text/x-python","patch_set":15,"id":"3a961159_0699a46c","line":57,"in_reply_to":"3a961159_e800fa36","updated":"2015-01-16 11:14:41.000000000","message":"OK. will try to figure out something","commit_id":"a8f0c20e1c895ae2a45aeedb1e4cfee5431bce45"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"8d20b31ee2d228a82d3d332c5b9cc5f77b9d07b4","unresolved":false,"context_lines":[{"line_number":68,"context_line":"                                   for tenant_id, groups in res.iteritems()))"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"def get_duplication_default_security_groups(engine):"},{"line_number":72,"context_line":"    # Models can change in time, but migration should rely only on exact"},{"line_number":73,"context_line":"    # model state at the current moment, so a separate model is created"},{"line_number":74,"context_line":"    # here."}],"source_content_type":"text/x-python","patch_set":17,"id":"3a961159_ab1f57b7","line":71,"updated":"2015-01-20 16:03:33.000000000","message":"duplication (a noun) -\u003e duplicate (an adjective)","commit_id":"beb602e21a253271d8d6df58796694a391da0dfa"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3e6bd0e08ec448e1c1f156ff0eb20e8146ac85e5","unresolved":false,"context_lines":[{"line_number":68,"context_line":"                                   for tenant_id, groups in res.iteritems()))"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"def get_duplication_default_security_groups(engine):"},{"line_number":72,"context_line":"    # Models can change in time, but migration should rely only on exact"},{"line_number":73,"context_line":"    # model state at the current moment, so a separate model is created"},{"line_number":74,"context_line":"    # here."}],"source_content_type":"text/x-python","patch_set":17,"id":"3a961159_3c6acbed","line":71,"in_reply_to":"3a961159_ab1f57b7","updated":"2015-01-20 17:06:19.000000000","message":"Oops","commit_id":"beb602e21a253271d8d6df58796694a391da0dfa"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c1332a23d009dd83a70bc5d43649544c7707bb68","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"class DuplicateSecurityGroupsNamedDefault(nexception.Conflict):"},{"line_number":36,"context_line":"    message \u003d _(\"There are more than one security group with name \u0027default\u0027 in\""},{"line_number":37,"context_line":"                \" some tenants: %(duplications)s. Database can\u0027t be upgraded.\""},{"line_number":38,"context_line":"                \"Duplications should be removed\")"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_8c4e1e95","line":37,"updated":"2015-01-22 15:00:08.000000000","message":"nit: space missing after the latest dot. Also, you could usefully add a dot at the very end of the message.","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"d92f47d25d67170e232d2efc9f7c817906e1942c","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"class DuplicateSecurityGroupsNamedDefault(nexception.Conflict):"},{"line_number":36,"context_line":"    message \u003d _(\"There are more than one security group with name \u0027default\u0027 in\""},{"line_number":37,"context_line":"                \" some tenants: %(duplications)s. Database can\u0027t be upgraded.\""},{"line_number":38,"context_line":"                \"Duplications should be removed\")"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_4f02680c","line":37,"in_reply_to":"1a930d6b_8c4e1e95","updated":"2015-01-22 15:13:32.000000000","message":"OK","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"95d55aad869d3481015bf69ad0ac894e3d79a706","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"def upgrade():"},{"line_number":42,"context_line":"    op.create_table(\u0027default_security_group\u0027,"},{"line_number":43,"context_line":"                    sa.Column(\u0027tenant_id\u0027, sa.String(length\u003d255),"},{"line_number":44,"context_line":"                              nullable\u003dFalse),"},{"line_number":45,"context_line":"                    sa.Column(\u0027security_group_id\u0027, sa.String(length\u003d36),"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_fe8e5bcf","line":42,"updated":"2015-01-22 19:13:32.000000000","message":"the Table object is returned here, with more recent Alembic versions....","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5f20c5c43bc76997233867ace891adc9e8921dc6","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"def upgrade():"},{"line_number":42,"context_line":"    op.create_table(\u0027default_security_group\u0027,"},{"line_number":43,"context_line":"                    sa.Column(\u0027tenant_id\u0027, sa.String(length\u003d255),"},{"line_number":44,"context_line":"                              nullable\u003dFalse),"},{"line_number":45,"context_line":"                    sa.Column(\u0027security_group_id\u0027, sa.String(length\u003d36),"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_442b0b30","line":42,"in_reply_to":"1a930d6b_fe8e5bcf","updated":"2015-01-23 11:23:53.000000000","message":"done","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"95d55aad869d3481015bf69ad0ac894e3d79a706","unresolved":false,"context_lines":[{"line_number":49,"context_line":"                                            [\u0027securitygroups.id\u0027],"},{"line_number":50,"context_line":"                                            ondelete\u003d\"CASCADE\")"},{"line_number":51,"context_line":"                    )"},{"line_number":52,"context_line":"    op.execute(\"INSERT INTO default_security_group \""},{"line_number":53,"context_line":"               \"(tenant_id, security_group_id) \""},{"line_number":54,"context_line":"               \"(select tenant_id, id \""},{"line_number":55,"context_line":"               \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_7eba6b6f","line":52,"updated":"2015-01-22 19:13:32.000000000","message":"... so you can use it here for the INSERT:   table.insert()...","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5f20c5c43bc76997233867ace891adc9e8921dc6","unresolved":false,"context_lines":[{"line_number":49,"context_line":"                                            [\u0027securitygroups.id\u0027],"},{"line_number":50,"context_line":"                                            ondelete\u003d\"CASCADE\")"},{"line_number":51,"context_line":"                    )"},{"line_number":52,"context_line":"    op.execute(\"INSERT INTO default_security_group \""},{"line_number":53,"context_line":"               \"(tenant_id, security_group_id) \""},{"line_number":54,"context_line":"               \"(select tenant_id, id \""},{"line_number":55,"context_line":"               \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_a41bcf19","line":52,"in_reply_to":"1a930d6b_7eba6b6f","updated":"2015-01-23 11:23:53.000000000","message":"done","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c1332a23d009dd83a70bc5d43649544c7707bb68","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    op.drop_table(\u0027default_security_group\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"def check_sanity(engine):"},{"line_number":63,"context_line":"    res \u003d get_duplicate_default_security_groups(engine)"},{"line_number":64,"context_line":"    if res:"},{"line_number":65,"context_line":"        raise DuplicateSecurityGroupsNamedDefault("}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_4f198878","line":62,"updated":"2015-01-22 15:00:08.000000000","message":"I appreciate you implemented it as a general mechanism. Thanks!","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"95d55aad869d3481015bf69ad0ac894e3d79a706","unresolved":false,"context_lines":[{"line_number":80,"context_line":"                                       nullable\u003dFalse),"},{"line_number":81,"context_line":"                             sa.Column(\u0027name\u0027, sa.String(255)),"},{"line_number":82,"context_line":"                             sa.Column(\u0027tenant_id\u0027, sa.String(255)))"},{"line_number":83,"context_line":"    Session \u003d sessionmaker(bind\u003dengine)"},{"line_number":84,"context_line":"    session \u003d Session(bind\u003dengine.connect())"},{"line_number":85,"context_line":"    subq \u003d (session.query(SecurityGroup)"},{"line_number":86,"context_line":"            .with_entities(SecurityGroup.c.tenant_id)"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_de06ff1f","line":83,"updated":"2015-01-22 19:13:32.000000000","message":"you can use sqlalchemy.orm.Session here directly, no need for sessionmaker","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5f20c5c43bc76997233867ace891adc9e8921dc6","unresolved":false,"context_lines":[{"line_number":80,"context_line":"                                       nullable\u003dFalse),"},{"line_number":81,"context_line":"                             sa.Column(\u0027name\u0027, sa.String(255)),"},{"line_number":82,"context_line":"                             sa.Column(\u0027tenant_id\u0027, sa.String(255)))"},{"line_number":83,"context_line":"    Session \u003d sessionmaker(bind\u003dengine)"},{"line_number":84,"context_line":"    session \u003d Session(bind\u003dengine.connect())"},{"line_number":85,"context_line":"    subq \u003d (session.query(SecurityGroup)"},{"line_number":86,"context_line":"            .with_entities(SecurityGroup.c.tenant_id)"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_24dfdf3c","line":83,"in_reply_to":"1a930d6b_de06ff1f","updated":"2015-01-23 11:23:53.000000000","message":"done","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"95d55aad869d3481015bf69ad0ac894e3d79a706","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                             sa.Column(\u0027tenant_id\u0027, sa.String(255)))"},{"line_number":83,"context_line":"    Session \u003d sessionmaker(bind\u003dengine)"},{"line_number":84,"context_line":"    session \u003d Session(bind\u003dengine.connect())"},{"line_number":85,"context_line":"    subq \u003d (session.query(SecurityGroup)"},{"line_number":86,"context_line":"            .with_entities(SecurityGroup.c.tenant_id)"},{"line_number":87,"context_line":"            .filter(SecurityGroup.c.name \u003d\u003d \u0027default\u0027)"},{"line_number":88,"context_line":"            .group_by(SecurityGroup.c.tenant_id)"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_695b972e","line":85,"updated":"2015-01-22 19:13:32.000000000","message":"you can just say session.query(SecurityGroup.c.tenant_id) here, no need for with_entities , but also SecurityGroup being CamelCase suggests it would be a class, I\u0027d suggest lowercasing it as this is a Table instance, not a class, e.g. security_group.   techincally you don\u0027t need the ORM at all here and could use conn.execute(table.select()) but using query and having join() might be convenient so that is OK.","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5f20c5c43bc76997233867ace891adc9e8921dc6","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                             sa.Column(\u0027tenant_id\u0027, sa.String(255)))"},{"line_number":83,"context_line":"    Session \u003d sessionmaker(bind\u003dengine)"},{"line_number":84,"context_line":"    session \u003d Session(bind\u003dengine.connect())"},{"line_number":85,"context_line":"    subq \u003d (session.query(SecurityGroup)"},{"line_number":86,"context_line":"            .with_entities(SecurityGroup.c.tenant_id)"},{"line_number":87,"context_line":"            .filter(SecurityGroup.c.name \u003d\u003d \u0027default\u0027)"},{"line_number":88,"context_line":"            .group_by(SecurityGroup.c.tenant_id)"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_64e807e3","line":85,"in_reply_to":"1a930d6b_695b972e","updated":"2015-01-23 11:23:53.000000000","message":"done","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"2df99726705e505e4c601bce8d1b70ab7f63aac7","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        table.insert().from_select([\u0027tenant_id\u0027, \u0027security_group_id\u0027],"},{"line_number":60,"context_line":"                                   sa.select([security_group.c.tenant_id,"},{"line_number":61,"context_line":"                                              security_group.c.id])"},{"line_number":62,"context_line":"                                   .where(security_group.c.name \u003d\u003d \u0027default\u0027)))"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"def downgrade():"}],"source_content_type":"text/x-python","patch_set":25,"id":"1a930d6b_3c144f43","line":62,"updated":"2015-01-23 13:17:16.000000000","message":"I not sure what actually went wrong http://docs.sqlalchemy.org/en/rel_0_9/changelog/migration_09.html#insert-from-select, but this variant does not work http://paste.openstack.org/show/160798/.","commit_id":"373dae46667cc2e29a142ca6bb69e2069640dfb0"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"c8fbf5856fc8353cd2bc36764b8220557439f04c","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        table.insert().from_select([\u0027tenant_id\u0027, \u0027security_group_id\u0027],"},{"line_number":60,"context_line":"                                   sa.select([security_group.c.tenant_id,"},{"line_number":61,"context_line":"                                              security_group.c.id])"},{"line_number":62,"context_line":"                                   .where(security_group.c.name \u003d\u003d \u0027default\u0027)))"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"def downgrade():"}],"source_content_type":"text/x-python","patch_set":25,"id":"1a930d6b_56f3af9d","line":62,"in_reply_to":"1a930d6b_3c144f43","updated":"2015-01-23 17:10:11.000000000","message":"OK there\u0027s a use contract with table.insert() such that we get the last inserted ID and other nice things, and on Postgresql theres a RETURNING that happens which actually returns rows.  that\u0027s not compatible with op.execute(), which is intended for DDL and other things that don\u0027t return results.\n\nSo in fact I\u0027m pretty sure we\u0027re hitting what\u0027s that big red warning at http://docs.sqlalchemy.org/en/rel_0_9/core/dml.html#sqlalchemy.sql.expression.Insert.from_select, which is fixed in 1.0: use the inline\u003dTrue flag on an INSERT from SELECT so that RETURNING is not appended to it:\n\n    sel \u003d select([table1.c.a, table1.c.b]).where(table1.c.c \u003e 5)\n    ins \u003d table2.insert(inline\u003dTrue).from_select([\u0027a\u0027, \u0027b\u0027], sel)\n\nthat should allow this to work with op.execute() as there will be no result set or \"last inserted id\" to fetch.","commit_id":"373dae46667cc2e29a142ca6bb69e2069640dfb0"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"0b0404d3310d516653d0607b79ebb14efc7c1fe3","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        table.insert().from_select([\u0027tenant_id\u0027, \u0027security_group_id\u0027],"},{"line_number":60,"context_line":"                                   sa.select([security_group.c.tenant_id,"},{"line_number":61,"context_line":"                                              security_group.c.id])"},{"line_number":62,"context_line":"                                   .where(security_group.c.name \u003d\u003d \u0027default\u0027)))"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"def downgrade():"}],"source_content_type":"text/x-python","patch_set":25,"id":"1a930d6b_c2d258ff","line":62,"in_reply_to":"1a930d6b_56f3af9d","updated":"2015-01-26 11:58:28.000000000","message":"That works! Thanks a lot for pointing this out!","commit_id":"373dae46667cc2e29a142ca6bb69e2069640dfb0"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"d729b552beb2c797ce079cefc56f8e3039a96267","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                                            [\u0027securitygroups.id\u0027],"},{"line_number":58,"context_line":"                                            ondelete\u003d\"CASCADE\")"},{"line_number":59,"context_line":"                    )"},{"line_number":60,"context_line":"    op.execute(\"INSERT INTO default_security_group \""},{"line_number":61,"context_line":"               \"(tenant_id, security_group_id) \""},{"line_number":62,"context_line":"               \"(select tenant_id, id \""},{"line_number":63,"context_line":"               \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a930d6b_f6101baa","line":60,"updated":"2015-01-23 17:11:07.000000000","message":"see my note on patch #25 for this.  I\u0027d like to see that new pattern working as it will serve as a model for other projects and such.","commit_id":"5bb0de4747979bf26d2dc731f1fe171837517aab"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"13cc01f0585735c9805ef58b9e478afa5b58b87b","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                                            [\u0027securitygroups.id\u0027],"},{"line_number":58,"context_line":"                                            ondelete\u003d\"CASCADE\")"},{"line_number":59,"context_line":"                    )"},{"line_number":60,"context_line":"    op.execute(\"INSERT INTO default_security_group \""},{"line_number":61,"context_line":"               \"(tenant_id, security_group_id) \""},{"line_number":62,"context_line":"               \"(select tenant_id, id \""},{"line_number":63,"context_line":"               \"FROM securitygroups WHERE name\u003d\u0027default\u0027)\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a930d6b_a201cc6b","line":60,"in_reply_to":"1a930d6b_f6101baa","updated":"2015-01-26 11:58:38.000000000","message":"done","commit_id":"5bb0de4747979bf26d2dc731f1fe171837517aab"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"ed180d6ae4c4f40d2ba610e4df6f2149d4bbe99d","unresolved":false,"context_lines":[{"line_number":43,"context_line":"class DuplicateSecurityGroupsNamedDefault(nexception.Conflict):"},{"line_number":44,"context_line":"    message \u003d _(\"There are more than one security group with name \u0027default\u0027 in\""},{"line_number":45,"context_line":"                \" some tenants: %(duplications)s. Database can\u0027t be upgraded. \""},{"line_number":46,"context_line":"                \"Duplications should be removed.\")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def upgrade():"}],"source_content_type":"text/x-python","patch_set":27,"id":"1a930d6b_912ece41","line":46,"updated":"2015-01-26 15:31:56.000000000","message":"\"Some tenants have more than one security group named \u0027default\u0027: %(duplications)s. All duplicate \u0027default\u0027 security groups must be resolved before upgrading the database.\"\n\nI don\u0027t think we should use the word \"removed\" because the duplication can be resolved by renaming.","commit_id":"274f29d7f7da338277f44ded3e17751646c4a5dd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"2066f6d3e5693195bc20cac932f4d22b02019c28","unresolved":false,"context_lines":[{"line_number":43,"context_line":"class DuplicateSecurityGroupsNamedDefault(nexception.Conflict):"},{"line_number":44,"context_line":"    message \u003d _(\"There are more than one security group with name \u0027default\u0027 in\""},{"line_number":45,"context_line":"                \" some tenants: %(duplications)s. Database can\u0027t be upgraded. \""},{"line_number":46,"context_line":"                \"Duplications should be removed.\")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def upgrade():"}],"source_content_type":"text/x-python","patch_set":27,"id":"1a930d6b_94a31cb1","line":46,"in_reply_to":"1a930d6b_912ece41","updated":"2015-01-26 15:52:55.000000000","message":"done","commit_id":"274f29d7f7da338277f44ded3e17751646c4a5dd"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"5ea778617643b25a06760af1b3cae0503c7bc3d8","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from alembic import op"},{"line_number":29,"context_line":"import sqlalchemy as sa"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"from neutron.common import exceptions as nexception"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"# Models can change in time, but migration should rely only on exact"},{"line_number":34,"context_line":"# model state at the current moment, so a separate model is created"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_a9812791","line":31,"updated":"2015-01-28 18:48:16.000000000","message":"(No action required) Consider using the same standards required for variable naming (lower, underscore-separated) for module aliases.","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a91912fca79513655b47daefff9be3104c785bb5","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from alembic import op"},{"line_number":29,"context_line":"import sqlalchemy as sa"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"from neutron.common import exceptions as nexception"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"# Models can change in time, but migration should rely only on exact"},{"line_number":34,"context_line":"# model state at the current moment, so a separate model is created"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_3d38f29f","line":31,"in_reply_to":"fa81d914_a9812791","updated":"2015-01-29 10:33:09.000000000","message":"renamed","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"f5bc8ebd5d80c5c3057f32fb1ac3b00d6bc87c98","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"# Models can change in time, but migration should rely only on exact"},{"line_number":34,"context_line":"# model state at the current moment, so a separate model is created"},{"line_number":35,"context_line":"# here."},{"line_number":36,"context_line":"security_group \u003d sa.Table(\u0027securitygroups\u0027, sa.MetaData(),"},{"line_number":37,"context_line":"                          sa.Column(\u0027id\u0027, sa.String(length\u003d36),"},{"line_number":38,"context_line":"                                    nullable\u003dFalse),"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_7409c330","line":35,"updated":"2015-01-28 19:49:58.000000000","message":"perfect.","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a91912fca79513655b47daefff9be3104c785bb5","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"# Models can change in time, but migration should rely only on exact"},{"line_number":34,"context_line":"# model state at the current moment, so a separate model is created"},{"line_number":35,"context_line":"# here."},{"line_number":36,"context_line":"security_group \u003d sa.Table(\u0027securitygroups\u0027, sa.MetaData(),"},{"line_number":37,"context_line":"                          sa.Column(\u0027id\u0027, sa.String(length\u003d36),"},{"line_number":38,"context_line":"                                    nullable\u003dFalse),"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_3d5f12e7","line":35,"in_reply_to":"fa81d914_7409c330","updated":"2015-01-29 10:33:09.000000000","message":"thanks!","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"5ea778617643b25a06760af1b3cae0503c7bc3d8","unresolved":false,"context_lines":[{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class DuplicateSecurityGroupsNamedDefault(nexception.Conflict):"},{"line_number":44,"context_line":"    message \u003d _(\"Some tenants have more than one security group named \""},{"line_number":45,"context_line":"                \"\u0027default\u0027: %(duplications)s. All duplicate \u0027default\u0027 security\""},{"line_number":46,"context_line":"                \" groups must be resolved before upgrading the database\")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_e9eebf8a","line":45,"updated":"2015-01-28 18:48:16.000000000","message":"(No action required) duplications -\u003e duplicates","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a91912fca79513655b47daefff9be3104c785bb5","unresolved":false,"context_lines":[{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class DuplicateSecurityGroupsNamedDefault(nexception.Conflict):"},{"line_number":44,"context_line":"    message \u003d _(\"Some tenants have more than one security group named \""},{"line_number":45,"context_line":"                \"\u0027default\u0027: %(duplications)s. All duplicate \u0027default\u0027 security\""},{"line_number":46,"context_line":"                \" groups must be resolved before upgrading the database\")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_9d5c7eef","line":45,"in_reply_to":"fa81d914_e9eebf8a","updated":"2015-01-29 10:33:09.000000000","message":"renamed","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"19e6773feb6f388f7a428c1b7554665a8db5d6d7","unresolved":false,"context_lines":[{"line_number":43,"context_line":"class DuplicateSecurityGroupsNamedDefault(nexception.Conflict):"},{"line_number":44,"context_line":"    message \u003d _(\"Some tenants have more than one security group named \""},{"line_number":45,"context_line":"                \"\u0027default\u0027: %(duplications)s. All duplicate \u0027default\u0027 security\""},{"line_number":46,"context_line":"                \" groups must be resolved before upgrading the database\")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def upgrade():"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_54120ab2","line":46,"updated":"2015-01-28 16:20:23.000000000","message":"nit: we miss dot at the end, again.","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"5ea778617643b25a06760af1b3cae0503c7bc3d8","unresolved":false,"context_lines":[{"line_number":43,"context_line":"class DuplicateSecurityGroupsNamedDefault(nexception.Conflict):"},{"line_number":44,"context_line":"    message \u003d _(\"Some tenants have more than one security group named \""},{"line_number":45,"context_line":"                \"\u0027default\u0027: %(duplications)s. All duplicate \u0027default\u0027 security\""},{"line_number":46,"context_line":"                \" groups must be resolved before upgrading the database\")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def upgrade():"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_e9bcff6e","line":46,"in_reply_to":"fa81d914_54120ab2","updated":"2015-01-28 18:48:16.000000000","message":"(No action required) it\u0027s usually easier to maintain strings with a consistent use of spacing across line-breaks.  The common pattern is to include the separating space at the end of the line rather than at the beginning.","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a91912fca79513655b47daefff9be3104c785bb5","unresolved":false,"context_lines":[{"line_number":43,"context_line":"class DuplicateSecurityGroupsNamedDefault(nexception.Conflict):"},{"line_number":44,"context_line":"    message \u003d _(\"Some tenants have more than one security group named \""},{"line_number":45,"context_line":"                \"\u0027default\u0027: %(duplications)s. All duplicate \u0027default\u0027 security\""},{"line_number":46,"context_line":"                \" groups must be resolved before upgrading the database\")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def upgrade():"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_fd6c9a9e","line":46,"in_reply_to":"fa81d914_54120ab2","updated":"2015-01-29 10:33:09.000000000","message":"done","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a91912fca79513655b47daefff9be3104c785bb5","unresolved":false,"context_lines":[{"line_number":43,"context_line":"class DuplicateSecurityGroupsNamedDefault(nexception.Conflict):"},{"line_number":44,"context_line":"    message \u003d _(\"Some tenants have more than one security group named \""},{"line_number":45,"context_line":"                \"\u0027default\u0027: %(duplications)s. All duplicate \u0027default\u0027 security\""},{"line_number":46,"context_line":"                \" groups must be resolved before upgrading the database\")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def upgrade():"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_1d688e86","line":46,"in_reply_to":"fa81d914_e9bcff6e","updated":"2015-01-29 10:33:09.000000000","message":"done","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"5ea778617643b25a06760af1b3cae0503c7bc3d8","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    res \u003d get_duplicate_default_security_groups(engine)"},{"line_number":72,"context_line":"    if res:"},{"line_number":73,"context_line":"        raise DuplicateSecurityGroupsNamedDefault("},{"line_number":74,"context_line":"            duplications\u003d\u0027; \u0027.join(\u0027tenant %s: %s\u0027 %"},{"line_number":75,"context_line":"                                   (tenant_id, \u0027, \u0027.join(groups))"},{"line_number":76,"context_line":"                                   for tenant_id, groups in res.iteritems()))"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_fb3126a3","line":74,"updated":"2015-01-28 18:48:16.000000000","message":"(No action required) duplications -\u003e duplicates","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a91912fca79513655b47daefff9be3104c785bb5","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    res \u003d get_duplicate_default_security_groups(engine)"},{"line_number":72,"context_line":"    if res:"},{"line_number":73,"context_line":"        raise DuplicateSecurityGroupsNamedDefault("},{"line_number":74,"context_line":"            duplications\u003d\u0027; \u0027.join(\u0027tenant %s: %s\u0027 %"},{"line_number":75,"context_line":"                                   (tenant_id, \u0027, \u0027.join(groups))"},{"line_number":76,"context_line":"                                   for tenant_id, groups in res.iteritems()))"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_3d0632cf","line":74,"in_reply_to":"fa81d914_fb3126a3","updated":"2015-01-29 10:33:09.000000000","message":"renamed","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"d3a7eb93a6d675a08c3e126d4ca2f2b9aec5b344","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from alembic import op"},{"line_number":29,"context_line":"import sqlalchemy as sa"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"from neutron.common import exceptions as exception"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"# Models can change in time, but migration should rely only on exact"},{"line_number":34,"context_line":"# model state at the current moment, so a separate model is created"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa81d914_db9efe53","line":31,"updated":"2015-01-29 10:47:03.000000000","message":"Sorry, now I don\u0027t follow at all why the alias.","commit_id":"a6fd0c9f9e31c4e7e248095ebfee5812895340bd"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"d3a7eb93a6d675a08c3e126d4ca2f2b9aec5b344","unresolved":false,"context_lines":[{"line_number":43,"context_line":"class DuplicateSecurityGroupsNamedDefault(exception.Conflict):"},{"line_number":44,"context_line":"    message \u003d _(\"Some tenants have more than one security group named \""},{"line_number":45,"context_line":"                \"\u0027default\u0027: %(duplicates)s. All duplicate \u0027default\u0027 security \""},{"line_number":46,"context_line":"                \"groups must be resolved before upgrading the database. \")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def upgrade():"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa81d914_9bb486d2","line":46,"updated":"2015-01-29 10:47:03.000000000","message":"nit: now we have redundant space at the end. :))","commit_id":"a6fd0c9f9e31c4e7e248095ebfee5812895340bd"}],"neutron/db/migration/cli.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"8d20b31ee2d228a82d3d332c5b9cc5f77b9d07b4","unresolved":false,"context_lines":[{"line_number":204,"context_line":"    for sc in script.iterate_revisions(revision, current_rev):"},{"line_number":205,"context_line":"        try:"},{"line_number":206,"context_line":"            sc.module.check_sanity(engine)"},{"line_number":207,"context_line":"        except AttributeError:"},{"line_number":208,"context_line":"            pass"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3a961159_567056a4","line":207,"updated":"2015-01-20 16:03:33.000000000","message":"This is too general an exception to check for against line 206.  An AttributeError thrown by the check_sanity call could be accidentally ignored.  Consider separating out attribute retrieval to minimize the scope of the exception handler, e.g.\n\ntry:\n check_sanity \u003d sc.module.check_sanity\nexcept AttributeError:\n continue\ncheck_sanity(engine)","commit_id":"beb602e21a253271d8d6df58796694a391da0dfa"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3e6bd0e08ec448e1c1f156ff0eb20e8146ac85e5","unresolved":false,"context_lines":[{"line_number":204,"context_line":"    for sc in script.iterate_revisions(revision, current_rev):"},{"line_number":205,"context_line":"        try:"},{"line_number":206,"context_line":"            sc.module.check_sanity(engine)"},{"line_number":207,"context_line":"        except AttributeError:"},{"line_number":208,"context_line":"            pass"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3a961159_bc54fb22","line":207,"in_reply_to":"3a961159_567056a4","updated":"2015-01-20 17:06:19.000000000","message":"OK, thanks","commit_id":"beb602e21a253271d8d6df58796694a391da0dfa"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"95d55aad869d3481015bf69ad0ac894e3d79a706","unresolved":false,"context_lines":[{"line_number":195,"context_line":""},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"def run_sanity_checks(config, revision):"},{"line_number":198,"context_line":"    engine \u003d sa.create_engine(config.neutron_config.database.connection)"},{"line_number":199,"context_line":"    script_dir \u003d alembic_script.ScriptDirectory.from_config(config)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def check_sanity(rev, context):"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_7e3fcbd6","line":198,"updated":"2015-01-22 19:13:32.000000000","message":"openstack projects need to get a lot better at sharing a single database connection / transaction along many alembic operations.  Alembic\u0027s API is a little scant here but there are ways to make it happen.   at the very least, I\u0027d produce a Connection here with engine.begin(), and pass that to check_sanity().  Calling execute() off the engine is not a great idea for non-trivial things.","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5f20c5c43bc76997233867ace891adc9e8921dc6","unresolved":false,"context_lines":[{"line_number":195,"context_line":""},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"def run_sanity_checks(config, revision):"},{"line_number":198,"context_line":"    engine \u003d sa.create_engine(config.neutron_config.database.connection)"},{"line_number":199,"context_line":"    script_dir \u003d alembic_script.ScriptDirectory.from_config(config)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def check_sanity(rev, context):"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_e4bb17e3","line":198,"in_reply_to":"1a930d6b_7e3fcbd6","updated":"2015-01-23 11:23:53.000000000","message":"done","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"d729b552beb2c797ce079cefc56f8e3039a96267","unresolved":false,"context_lines":[{"line_number":208,"context_line":"                                             fn\u003dcheck_sanity,"},{"line_number":209,"context_line":"                                             starting_rev\u003dNone,"},{"line_number":210,"context_line":"                                             destination_rev\u003drevision)"},{"line_number":211,"context_line":"    with context, engine.begin() as connection:"},{"line_number":212,"context_line":"        script_dir.run_env()"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"1a930d6b_d6debf3e","line":211,"updated":"2015-01-23 17:11:07.000000000","message":"oh, hmmm.  yes this works but i think your env.py is still going to make its own connection, isn\u0027t it.     OK check_sanity here is being run underneath EnvironmentContext (was it like that before?  hm), so it should use the bind that\u0027s already set up here.    so above:\n\n    script.module.check_sanity(context.connection)\n\nthen you don\u0027t need the engine.begin() here, that already happens effectively within run_env()","commit_id":"5bb0de4747979bf26d2dc731f1fe171837517aab"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"13cc01f0585735c9805ef58b9e478afa5b58b87b","unresolved":false,"context_lines":[{"line_number":208,"context_line":"                                             fn\u003dcheck_sanity,"},{"line_number":209,"context_line":"                                             starting_rev\u003dNone,"},{"line_number":210,"context_line":"                                             destination_rev\u003drevision)"},{"line_number":211,"context_line":"    with context, engine.begin() as connection:"},{"line_number":212,"context_line":"        script_dir.run_env()"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"1a930d6b_b9f65bf7","line":211,"in_reply_to":"1a930d6b_d6debf3e","updated":"2015-01-26 11:58:38.000000000","message":"done","commit_id":"5bb0de4747979bf26d2dc731f1fe171837517aab"}],"neutron/db/securitygroups_db.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"28436f22e743f0385f3056d86427bc7c46711af6","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    description \u003d sa.Column(sa.String(255))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class DefaultSecurityGroup(model_base.BASEV2):"},{"line_number":44,"context_line":"    tenant_id \u003d sa.Column(sa.String(255), primary_key\u003dTrue, nullable\u003dFalse)"},{"line_number":45,"context_line":"    security_group_id \u003d sa.Column(sa.String(36), nullable\u003dFalse)"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_4d4c95c8","line":43,"updated":"2014-12-23 21:05:25.000000000","message":"Is there a reason there aren\u0027t foreign key constraints (with ondelete\u003dCASCADE) on both of the columns in this table?","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"b493307e5d282359c678a7148ca6e432c54d3db1","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    description \u003d sa.Column(sa.String(255))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class DefaultSecurityGroup(model_base.BASEV2):"},{"line_number":44,"context_line":"    tenant_id \u003d sa.Column(sa.String(255), primary_key\u003dTrue, nullable\u003dFalse)"},{"line_number":45,"context_line":"    security_group_id \u003d sa.Column(sa.String(36), nullable\u003dFalse)"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_b169c1bf","line":43,"in_reply_to":"3a961159_4d4c95c8","updated":"2014-12-24 12:48:10.000000000","message":"You suggested this :) I based on http://paste.openstack.org/show/148903/ that you provide as example.","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4b4213f2bd9a6de6940e71c8e97e7461b4d542f5","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    description \u003d sa.Column(sa.String(255))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class DefaultSecurityGroup(model_base.BASEV2):"},{"line_number":44,"context_line":"    tenant_id \u003d sa.Column(sa.String(255), primary_key\u003dTrue, nullable\u003dFalse)"},{"line_number":45,"context_line":"    security_group_id \u003d sa.Column(sa.String(36), nullable\u003dFalse)"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_d569ce8d","line":43,"in_reply_to":"3a961159_b169c1bf","updated":"2014-12-25 03:35:13.000000000","message":"Fair enough.  I know that I (and jaypipes, who kindly provided the example) am just as much in need of review cycles as everyone else.  :)","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"28436f22e743f0385f3056d86427bc7c46711af6","unresolved":false,"context_lines":[{"line_number":123,"context_line":"                                              tenant_id\u003dtenant_id,"},{"line_number":124,"context_line":"                                              name\u003ds[\u0027name\u0027])"},{"line_number":125,"context_line":"            context.session.add(security_group_db)"},{"line_number":126,"context_line":"            if s.get(\u0027name\u0027) \u003d\u003d \u0027default\u0027:"},{"line_number":127,"context_line":"                context.session.add(DefaultSecurityGroup("},{"line_number":128,"context_line":"                    security_group_id\u003dsecurity_group_db[\u0027id\u0027],"},{"line_number":129,"context_line":"                    tenant_id\u003dsecurity_group_db[\u0027tenant_id\u0027]))"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_cd6c05d3","line":126,"updated":"2014-12-23 21:05:25.000000000","message":"default_sg will equal True here, so please use that instead.\n\nAlso, since we\u0027re not adding a uniqueness constraint on the name, we can\u0027t key on the name to identify the default group.","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"48b7d93acdb68cfe89502d01646d0ee5d3968c67","unresolved":false,"context_lines":[{"line_number":123,"context_line":"                                              tenant_id\u003dtenant_id,"},{"line_number":124,"context_line":"                                              name\u003ds[\u0027name\u0027])"},{"line_number":125,"context_line":"            context.session.add(security_group_db)"},{"line_number":126,"context_line":"            if s.get(\u0027name\u0027) \u003d\u003d \u0027default\u0027:"},{"line_number":127,"context_line":"                context.session.add(DefaultSecurityGroup("},{"line_number":128,"context_line":"                    security_group_id\u003dsecurity_group_db[\u0027id\u0027],"},{"line_number":129,"context_line":"                    tenant_id\u003dsecurity_group_db[\u0027tenant_id\u0027]))"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_f6bb2d0d","line":126,"in_reply_to":"3a961159_cd6c05d3","updated":"2014-12-25 12:29:51.000000000","message":"done","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"28436f22e743f0385f3056d86427bc7c46711af6","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                    security_group_id\u003dsecurity_group_db[\u0027id\u0027],"},{"line_number":129,"context_line":"                    tenant_id\u003dsecurity_group_db[\u0027tenant_id\u0027]))"},{"line_number":130,"context_line":"            for ethertype in ext_sg.sg_supported_ethertypes:"},{"line_number":131,"context_line":"                if s.get(\u0027name\u0027) \u003d\u003d \u0027default\u0027:"},{"line_number":132,"context_line":"                    # Allow intercommunication"},{"line_number":133,"context_line":"                    ingress_rule \u003d SecurityGroupRule("},{"line_number":134,"context_line":"                        id\u003duuidutils.generate_uuid(), tenant_id\u003dtenant_id,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_adcc1936","line":131,"updated":"2014-12-23 21:05:25.000000000","message":"Please use default_sg here.","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"48b7d93acdb68cfe89502d01646d0ee5d3968c67","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                    security_group_id\u003dsecurity_group_db[\u0027id\u0027],"},{"line_number":129,"context_line":"                    tenant_id\u003dsecurity_group_db[\u0027tenant_id\u0027]))"},{"line_number":130,"context_line":"            for ethertype in ext_sg.sg_supported_ethertypes:"},{"line_number":131,"context_line":"                if s.get(\u0027name\u0027) \u003d\u003d \u0027default\u0027:"},{"line_number":132,"context_line":"                    # Allow intercommunication"},{"line_number":133,"context_line":"                    ingress_rule \u003d SecurityGroupRule("},{"line_number":134,"context_line":"                        id\u003duuidutils.generate_uuid(), tenant_id\u003dtenant_id,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_16b99915","line":131,"in_reply_to":"3a961159_adcc1936","updated":"2014-12-25 12:29:51.000000000","message":"done","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"28436f22e743f0385f3056d86427bc7c46711af6","unresolved":false,"context_lines":[{"line_number":164,"context_line":"            self._ensure_default_security_group(context, tenant_id)"},{"line_number":165,"context_line":"        marker_obj \u003d self._get_marker_obj(context, \u0027security_group\u0027, limit,"},{"line_number":166,"context_line":"                                          marker)"},{"line_number":167,"context_line":"        if default_sg:"},{"line_number":168,"context_line":"            default_sgs \u003d self._get_collection("},{"line_number":169,"context_line":"                context, DefaultSecurityGroup,"},{"line_number":170,"context_line":"                self._make_default_security_group_dict,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_df3bc8ce","line":167,"updated":"2014-12-23 21:05:25.000000000","message":"The intent here appears to be to return the default security group if default_sg is True.  That would mean that default_sg has 2 very different purposes - indicating whether or not to ensure that the default security group exists, and indicating whether to return only the default security group - and I think this kind of overloading is confusing.","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"48b7d93acdb68cfe89502d01646d0ee5d3968c67","unresolved":false,"context_lines":[{"line_number":164,"context_line":"            self._ensure_default_security_group(context, tenant_id)"},{"line_number":165,"context_line":"        marker_obj \u003d self._get_marker_obj(context, \u0027security_group\u0027, limit,"},{"line_number":166,"context_line":"                                          marker)"},{"line_number":167,"context_line":"        if default_sg:"},{"line_number":168,"context_line":"            default_sgs \u003d self._get_collection("},{"line_number":169,"context_line":"                context, DefaultSecurityGroup,"},{"line_number":170,"context_line":"                self._make_default_security_group_dict,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_d6be3100","line":167,"in_reply_to":"3a961159_df3bc8ce","updated":"2014-12-25 12:29:51.000000000","message":"I removed this","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"28436f22e743f0385f3056d86427bc7c46711af6","unresolved":false,"context_lines":[{"line_number":212,"context_line":"            raise ext_sg.SecurityGroupNotFound(id\u003did)"},{"line_number":213,"context_line":"        return sg"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def _get_def_security_group(self, context, id):"},{"line_number":216,"context_line":"        try:"},{"line_number":217,"context_line":"            query \u003d self._model_query(context, DefaultSecurityGroup)"},{"line_number":218,"context_line":"            sg \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_8d391da6","line":215,"updated":"2014-12-23 21:05:25.000000000","message":"Please avoid using obscure acronyms.  In this case \u0027default\u0027 would be preferable to \u0027def\u0027, especially given that def could be ambiguous (defined, default, python keyword, etc).","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"b493307e5d282359c678a7148ca6e432c54d3db1","unresolved":false,"context_lines":[{"line_number":212,"context_line":"            raise ext_sg.SecurityGroupNotFound(id\u003did)"},{"line_number":213,"context_line":"        return sg"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def _get_def_security_group(self, context, id):"},{"line_number":216,"context_line":"        try:"},{"line_number":217,"context_line":"            query \u003d self._model_query(context, DefaultSecurityGroup)"},{"line_number":218,"context_line":"            sg \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_718039f7","line":215,"in_reply_to":"3a961159_8d391da6","updated":"2014-12-24 12:48:10.000000000","message":"Oops, forget to change this from WIP version.","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"28436f22e743f0385f3056d86427bc7c46711af6","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        # confirm security group exists"},{"line_number":231,"context_line":"        sg \u003d self._get_security_group(context, id)"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"        if sg[\u0027name\u0027] \u003d\u003d \u0027default\u0027 and not context.is_admin:"},{"line_number":234,"context_line":"            raise ext_sg.SecurityGroupCannotRemoveDefault()"},{"line_number":235,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":236,"context_line":"            context.session.delete(sg)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_edac61a1","line":233,"updated":"2014-12-23 21:05:25.000000000","message":"As per a previous comment, we can\u0027t rely on the name to identify the default group.  Instead, DefaultSecurityGroup will have to be consulted, e.g.\n\n\ntry:\n  default_group \u003d self.get_default_security_group(context, id)\nexcept ext_sg.SecurityGroupNotFound:\n  is_default \u003d False\nelse:\n  is_default \u003d True\n\nif is_default and not context.is_admin:\n  raise\nwith\n  context.session.delete(sg)\n  if is_default:\n     context.session.delete(default_group)","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"b493307e5d282359c678a7148ca6e432c54d3db1","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        # confirm security group exists"},{"line_number":231,"context_line":"        sg \u003d self._get_security_group(context, id)"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"        if sg[\u0027name\u0027] \u003d\u003d \u0027default\u0027 and not context.is_admin:"},{"line_number":234,"context_line":"            raise ext_sg.SecurityGroupCannotRemoveDefault()"},{"line_number":235,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":236,"context_line":"            context.session.delete(sg)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_d1f48d88","line":233,"in_reply_to":"3a961159_edac61a1","updated":"2014-12-24 12:48:10.000000000","message":"OK","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"28436f22e743f0385f3056d86427bc7c46711af6","unresolved":false,"context_lines":[{"line_number":538,"context_line":"        :returns: the default security group id."},{"line_number":539,"context_line":"        \"\"\""},{"line_number":540,"context_line":"        filters \u003d {\u0027tenant_id\u0027: [tenant_id]}"},{"line_number":541,"context_line":"        default_group \u003d self._get_collection("},{"line_number":542,"context_line":"            context, DefaultSecurityGroup,"},{"line_number":543,"context_line":"            self._make_default_security_group_dict,"},{"line_number":544,"context_line":"            filters\u003dfilters)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_4dd9f520","line":541,"updated":"2014-12-23 21:05:25.000000000","message":"Is there a reason not to query the table directly?  It seems wasteful to invoke _get_collection where query.one() would suffice.","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"b493307e5d282359c678a7148ca6e432c54d3db1","unresolved":false,"context_lines":[{"line_number":538,"context_line":"        :returns: the default security group id."},{"line_number":539,"context_line":"        \"\"\""},{"line_number":540,"context_line":"        filters \u003d {\u0027tenant_id\u0027: [tenant_id]}"},{"line_number":541,"context_line":"        default_group \u003d self._get_collection("},{"line_number":542,"context_line":"            context, DefaultSecurityGroup,"},{"line_number":543,"context_line":"            self._make_default_security_group_dict,"},{"line_number":544,"context_line":"            filters\u003dfilters)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_91ff6565","line":541,"in_reply_to":"3a961159_4dd9f520","updated":"2014-12-24 12:48:10.000000000","message":"OK","commit_id":"b68df08c2488d0a96a4e6c6ccbe7b0b25d137f7d"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"aaa876c61bbd4bf3b49a29e4e2de04c6538bb9f0","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    __table_name__ \u003d \u0027default_security_group\u0027"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    tenant_id \u003d sa.Column(sa.String(255), primary_key\u003dTrue, nullable\u003dFalse)"},{"line_number":47,"context_line":"    security_group_id \u003d sa.Column(sa.String(36), nullable\u003dFalse)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"class SecurityGroupPortBinding(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_0cfeecae","line":47,"updated":"2014-12-25 15:03:56.000000000","message":"Is this worthwhile having a cascading deletion here?","commit_id":"03f8221c83f9cc32a4f5f4df12c2d8d21a942758"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a44f0b3dfd3a58c01c2044fee2811da8805ce5c8","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    __table_name__ \u003d \u0027default_security_group\u0027"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    tenant_id \u003d sa.Column(sa.String(255), primary_key\u003dTrue, nullable\u003dFalse)"},{"line_number":47,"context_line":"    security_group_id \u003d sa.Column(sa.String(36), nullable\u003dFalse)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"class SecurityGroupPortBinding(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_92c82aaa","line":47,"in_reply_to":"3a961159_0cfeecae","updated":"2014-12-26 11:56:39.000000000","message":"Will add fk on this column in next patch set","commit_id":"03f8221c83f9cc32a4f5f4df12c2d8d21a942758"},{"author":{"_account_id":7962,"name":"Ronak Shah","email":"ronak.malav.shah@gmail.com","username":"ronak"},"change_message_id":"afb08b13d35862463d82d1f63e781ee0c0bbe130","unresolved":false,"context_lines":[{"line_number":215,"context_line":"            raise ext_sg.SecurityGroupNotFound(id\u003did)"},{"line_number":216,"context_line":"        return sg"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def _get_default_security_group(self, context, id):"},{"line_number":219,"context_line":"        try:"},{"line_number":220,"context_line":"            query \u003d self._model_query(context, DefaultSecurityGroup)"},{"line_number":221,"context_line":"            sg \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_7dc1517f","line":218,"updated":"2015-01-06 07:15:47.000000000","message":"would it be more appropriate to rename this to _get_default_security_group_by_sgid instead since security_group_id is not primary key to the model being queried?","commit_id":"c3f094ead6d12cfa7a11c178ecd0da935f2b0c93"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"32b16b8bc76b7089693bf7f047bdb037389e7935","unresolved":false,"context_lines":[{"line_number":215,"context_line":"            raise ext_sg.SecurityGroupNotFound(id\u003did)"},{"line_number":216,"context_line":"        return sg"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def _get_default_security_group(self, context, id):"},{"line_number":219,"context_line":"        try:"},{"line_number":220,"context_line":"            query \u003d self._model_query(context, DefaultSecurityGroup)"},{"line_number":221,"context_line":"            sg \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_1f151349","line":218,"in_reply_to":"3a961159_79ef17b2","updated":"2015-01-12 11:13:05.000000000","message":"remove this method as it is not needed","commit_id":"c3f094ead6d12cfa7a11c178ecd0da935f2b0c93"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"4ac2287defe1cdcf730dd33e23b5a1dcfa0e8453","unresolved":false,"context_lines":[{"line_number":215,"context_line":"            raise ext_sg.SecurityGroupNotFound(id\u003did)"},{"line_number":216,"context_line":"        return sg"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def _get_default_security_group(self, context, id):"},{"line_number":219,"context_line":"        try:"},{"line_number":220,"context_line":"            query \u003d self._model_query(context, DefaultSecurityGroup)"},{"line_number":221,"context_line":"            sg \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_79ef17b2","line":218,"in_reply_to":"3a961159_7dc1517f","updated":"2015-01-12 11:00:37.000000000","message":"done","commit_id":"c3f094ead6d12cfa7a11c178ecd0da935f2b0c93"},{"author":{"_account_id":7962,"name":"Ronak Shah","email":"ronak.malav.shah@gmail.com","username":"ronak"},"change_message_id":"afb08b13d35862463d82d1f63e781ee0c0bbe130","unresolved":false,"context_lines":[{"line_number":541,"context_line":"        try:"},{"line_number":542,"context_line":"            default_group \u003d query.filter("},{"line_number":543,"context_line":"                DefaultSecurityGroup.tenant_id \u003d\u003d tenant_id).one()"},{"line_number":544,"context_line":"        except exc.NoResultFound:"},{"line_number":545,"context_line":"            security_group \u003d {"},{"line_number":546,"context_line":"                \u0027security_group\u0027: {\u0027name\u0027: \u0027default\u0027,"},{"line_number":547,"context_line":"                                   \u0027tenant_id\u0027: tenant_id,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_9de35d33","line":544,"updated":"2015-01-06 07:15:47.000000000","message":"It would be useful to make above code a method just like _get_default_security_group","commit_id":"c3f094ead6d12cfa7a11c178ecd0da935f2b0c93"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"4ac2287defe1cdcf730dd33e23b5a1dcfa0e8453","unresolved":false,"context_lines":[{"line_number":541,"context_line":"        try:"},{"line_number":542,"context_line":"            default_group \u003d query.filter("},{"line_number":543,"context_line":"                DefaultSecurityGroup.tenant_id \u003d\u003d tenant_id).one()"},{"line_number":544,"context_line":"        except exc.NoResultFound:"},{"line_number":545,"context_line":"            security_group \u003d {"},{"line_number":546,"context_line":"                \u0027security_group\u0027: {\u0027name\u0027: \u0027default\u0027,"},{"line_number":547,"context_line":"                                   \u0027tenant_id\u0027: tenant_id,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_5f9ddb56","line":544,"in_reply_to":"3a961159_9de35d33","updated":"2015-01-12 11:00:37.000000000","message":"why? tenant_id is passed to this method.","commit_id":"c3f094ead6d12cfa7a11c178ecd0da935f2b0c93"},{"author":{"_account_id":7962,"name":"Ronak Shah","email":"ronak.malav.shah@gmail.com","username":"ronak"},"change_message_id":"afb08b13d35862463d82d1f63e781ee0c0bbe130","unresolved":false,"context_lines":[{"line_number":547,"context_line":"                                   \u0027tenant_id\u0027: tenant_id,"},{"line_number":548,"context_line":"                                   \u0027description\u0027: _(\u0027Default security group\u0027)}"},{"line_number":549,"context_line":"            }"},{"line_number":550,"context_line":"            ret \u003d self.create_security_group(context, security_group, True)"},{"line_number":551,"context_line":"            return ret[\u0027id\u0027]"},{"line_number":552,"context_line":"        else:"},{"line_number":553,"context_line":"            return default_group[\u0027security_group_id\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_7d9311a3","line":550,"updated":"2015-01-06 07:15:47.000000000","message":"default_sg\u003dTrue as third parameter","commit_id":"c3f094ead6d12cfa7a11c178ecd0da935f2b0c93"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"4ac2287defe1cdcf730dd33e23b5a1dcfa0e8453","unresolved":false,"context_lines":[{"line_number":547,"context_line":"                                   \u0027tenant_id\u0027: tenant_id,"},{"line_number":548,"context_line":"                                   \u0027description\u0027: _(\u0027Default security group\u0027)}"},{"line_number":549,"context_line":"            }"},{"line_number":550,"context_line":"            ret \u003d self.create_security_group(context, security_group, True)"},{"line_number":551,"context_line":"            return ret[\u0027id\u0027]"},{"line_number":552,"context_line":"        else:"},{"line_number":553,"context_line":"            return default_group[\u0027security_group_id\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_3ff56f84","line":550,"in_reply_to":"3a961159_7d9311a3","updated":"2015-01-12 11:00:37.000000000","message":"In fact it won\u0027t be relevant change here, but will change this.","commit_id":"c3f094ead6d12cfa7a11c178ecd0da935f2b0c93"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1cc54f97df35ac9500e2f12ef0ac567d9d31d386","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        sg \u003d self._get_security_group(context, id)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"        if sg.default_security_group and not context.is_admin:"},{"line_number":227,"context_line":"            raise ext_sg.SecurityGroupCannotRemoveDefault()"},{"line_number":228,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":229,"context_line":"            context.session.delete(sg)"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_502ce5ac","line":227,"updated":"2015-01-12 14:05:42.000000000","message":"Not particularly relevant to the bug, but why do we allow to delete default security group for admins?","commit_id":"0c4c8919bc27ca7fdb0b5efa0ef8c99641cea0ed"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"7d44e58f1cea9226bc7df071beae1d35ed3ba7f5","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        sg \u003d self._get_security_group(context, id)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"        if sg.default_security_group and not context.is_admin:"},{"line_number":227,"context_line":"            raise ext_sg.SecurityGroupCannotRemoveDefault()"},{"line_number":228,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":229,"context_line":"            context.session.delete(sg)"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_8bda7777","line":227,"in_reply_to":"3a961159_502ce5ac","updated":"2015-01-13 14:00:34.000000000","message":"At now default security group is not deleted when a corresponding tenant is deleted in keystone. This means that the deleted tenant can no longer delete its security group. Only admin has a priveledge to operate resources of other tenants, so I think we need to allow for admin to delete default security groups.","commit_id":"0c4c8919bc27ca7fdb0b5efa0ef8c99641cea0ed"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"452324ac14e89a1b9505c4f7e820bf28cefa2b01","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        sg \u003d self._get_security_group(context, id)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"        if sg.default_security_group and not context.is_admin:"},{"line_number":227,"context_line":"            raise ext_sg.SecurityGroupCannotRemoveDefault()"},{"line_number":228,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":229,"context_line":"            context.session.delete(sg)"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_2b5b6e56","line":227,"in_reply_to":"3a961159_502ce5ac","updated":"2015-01-12 14:27:15.000000000","message":"I don\u0027t think about it in this way and this is an interesting point. Admin has high level to access for all objects that is why it is allowed for him to delete default security group. May be it is because it could be necessary to set default security group different from that we have in common.","commit_id":"0c4c8919bc27ca7fdb0b5efa0ef8c99641cea0ed"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f1bb14f06b10d38e57091e39f4f6a0de8d1bf5b3","unresolved":false,"context_lines":[{"line_number":48,"context_line":"                                  sa.ForeignKey(\"securitygroups.id\","},{"line_number":49,"context_line":"                                                ondelete\u003d\"CASCADE\"),"},{"line_number":50,"context_line":"                                  nullable\u003dFalse)"},{"line_number":51,"context_line":"    security_group \u003d orm.relationship("},{"line_number":52,"context_line":"        SecurityGroup, lazy\u003d\u0027joined\u0027,"},{"line_number":53,"context_line":"        backref\u003dorm.backref(\u0027default_security_group\u0027, cascade\u003d\u0027all,delete\u0027),"},{"line_number":54,"context_line":"        primaryjoin\u003d\"SecurityGroup.id\u003d\u003dDefaultSecurityGroup.security_group_id\","}],"source_content_type":"text/x-python","patch_set":15,"id":"3a961159_e37adb23","line":51,"updated":"2015-01-15 20:40:59.000000000","message":"Clever!","commit_id":"a8f0c20e1c895ae2a45aeedb1e4cfee5431bce45"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"312092f34d18aa242fd5af7252d1603c6faa9d58","unresolved":false,"context_lines":[{"line_number":48,"context_line":"                                  sa.ForeignKey(\"securitygroups.id\","},{"line_number":49,"context_line":"                                                ondelete\u003d\"CASCADE\"),"},{"line_number":50,"context_line":"                                  nullable\u003dFalse)"},{"line_number":51,"context_line":"    security_group \u003d orm.relationship("},{"line_number":52,"context_line":"        SecurityGroup, lazy\u003d\u0027joined\u0027,"},{"line_number":53,"context_line":"        backref\u003dorm.backref(\u0027default_security_group\u0027, cascade\u003d\u0027all,delete\u0027),"},{"line_number":54,"context_line":"        primaryjoin\u003d\"SecurityGroup.id\u003d\u003dDefaultSecurityGroup.security_group_id\","}],"source_content_type":"text/x-python","patch_set":15,"id":"3a961159_06b0c4da","line":51,"in_reply_to":"3a961159_e37adb23","updated":"2015-01-16 11:14:41.000000000","message":"thanks! :)","commit_id":"a8f0c20e1c895ae2a45aeedb1e4cfee5431bce45"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f1bb14f06b10d38e57091e39f4f6a0de8d1bf5b3","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        s \u003d security_group[\u0027security_group\u0027]"},{"line_number":233,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":234,"context_line":"            sg \u003d self._get_security_group(context, id)"},{"line_number":235,"context_line":"            if sg[\u0027name\u0027] \u003d\u003d \u0027default\u0027 and \u0027name\u0027 in s:"},{"line_number":236,"context_line":"                raise ext_sg.SecurityGroupCannotUpdateDefault()"},{"line_number":237,"context_line":"            sg.update(s)"},{"line_number":238,"context_line":"        return self._make_security_group_dict(sg)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3a961159_0392879e","line":235,"updated":"2015-01-15 20:40:59.000000000","message":"This should probably be changed to use the \u0027default_security_group\u0027 property as on line 226.","commit_id":"a8f0c20e1c895ae2a45aeedb1e4cfee5431bce45"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"312092f34d18aa242fd5af7252d1603c6faa9d58","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        s \u003d security_group[\u0027security_group\u0027]"},{"line_number":233,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":234,"context_line":"            sg \u003d self._get_security_group(context, id)"},{"line_number":235,"context_line":"            if sg[\u0027name\u0027] \u003d\u003d \u0027default\u0027 and \u0027name\u0027 in s:"},{"line_number":236,"context_line":"                raise ext_sg.SecurityGroupCannotUpdateDefault()"},{"line_number":237,"context_line":"            sg.update(s)"},{"line_number":238,"context_line":"        return self._make_security_group_dict(sg)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3a961159_6672684b","line":235,"in_reply_to":"3a961159_0392879e","updated":"2015-01-16 11:14:41.000000000","message":"done","commit_id":"a8f0c20e1c895ae2a45aeedb1e4cfee5431bce45"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f1bb14f06b10d38e57091e39f4f6a0de8d1bf5b3","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                                       for r in security_group.rules]"},{"line_number":247,"context_line":"        return self._fields(res, fields)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def _make_default_security_group_dict(self, security_group, fields\u003dNone):"},{"line_number":250,"context_line":"        res \u003d {\u0027security_group_id\u0027: security_group[\u0027security_group_id\u0027],"},{"line_number":251,"context_line":"               \u0027tenant_id\u0027: security_group[\u0027tenant_id\u0027]}"},{"line_number":252,"context_line":"        return self._fields(res, fields)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3a961159_881cd678","line":249,"updated":"2015-01-15 20:40:59.000000000","message":"This appears to be unused.","commit_id":"a8f0c20e1c895ae2a45aeedb1e4cfee5431bce45"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"312092f34d18aa242fd5af7252d1603c6faa9d58","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                                       for r in security_group.rules]"},{"line_number":247,"context_line":"        return self._fields(res, fields)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def _make_default_security_group_dict(self, security_group, fields\u003dNone):"},{"line_number":250,"context_line":"        res \u003d {\u0027security_group_id\u0027: security_group[\u0027security_group_id\u0027],"},{"line_number":251,"context_line":"               \u0027tenant_id\u0027: security_group[\u0027tenant_id\u0027]}"},{"line_number":252,"context_line":"        return self._fields(res, fields)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3a961159_267ce01f","line":249,"in_reply_to":"3a961159_881cd678","updated":"2015-01-16 11:14:41.000000000","message":"done","commit_id":"a8f0c20e1c895ae2a45aeedb1e4cfee5431bce45"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ea6c6574fe2ad03d0556cebd30878e75bf431fd1","unresolved":false,"context_lines":[{"line_number":242,"context_line":"               \u0027name\u0027: security_group[\u0027name\u0027],"},{"line_number":243,"context_line":"               \u0027tenant_id\u0027: security_group[\u0027tenant_id\u0027],"},{"line_number":244,"context_line":"               \u0027description\u0027: security_group[\u0027description\u0027],"},{"line_number":245,"context_line":"               \u0027default\u0027: bool(security_group.default_security_group)}"},{"line_number":246,"context_line":"        res[\u0027security_group_rules\u0027] \u003d [self._make_security_group_rule_dict(r)"},{"line_number":247,"context_line":"                                       for r in security_group.rules]"},{"line_number":248,"context_line":"        return self._fields(res, fields)"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_68ff4e03","line":245,"updated":"2015-01-19 23:53:30.000000000","message":"I can see how this line was suggested by my asking you to fix the plumgrid and vmware plugins usage of the magic string \u0027default\u0027.  Adding this field to the dict serialization will ensure that clients see it, too, and I\u0027m not sure we want that.  As per my comment on the last patchset, the name of the default security group has to be \u0027default\u0027 for the sake of Nova, so consider reverting back to the old way of detecting the default group (e.g. back to using sg[\u0027name\u0027] \u003d\u003d \u0027default\u0027).\n\nI apologize for leading you around in circles on this issue.  I didn\u0027t really understand the interaction with Nova at first.","commit_id":"e91fbea5ea29edd58efe632f442bb11508345221"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"8d20b31ee2d228a82d3d332c5b9cc5f77b9d07b4","unresolved":false,"context_lines":[{"line_number":242,"context_line":"               \u0027name\u0027: security_group[\u0027name\u0027],"},{"line_number":243,"context_line":"               \u0027tenant_id\u0027: security_group[\u0027tenant_id\u0027],"},{"line_number":244,"context_line":"               \u0027description\u0027: security_group[\u0027description\u0027],"},{"line_number":245,"context_line":"               \u0027default\u0027: bool(security_group.default_security_group)}"},{"line_number":246,"context_line":"        res[\u0027security_group_rules\u0027] \u003d [self._make_security_group_rule_dict(r)"},{"line_number":247,"context_line":"                                       for r in security_group.rules]"},{"line_number":248,"context_line":"        return self._fields(res, fields)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3a961159_b6b5fa33","line":245,"updated":"2015-01-20 16:03:33.000000000","message":"Any thoughts as to my comment on this on the last patchset?","commit_id":"beb602e21a253271d8d6df58796694a391da0dfa"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3e6bd0e08ec448e1c1f156ff0eb20e8146ac85e5","unresolved":false,"context_lines":[{"line_number":242,"context_line":"               \u0027name\u0027: security_group[\u0027name\u0027],"},{"line_number":243,"context_line":"               \u0027tenant_id\u0027: security_group[\u0027tenant_id\u0027],"},{"line_number":244,"context_line":"               \u0027description\u0027: security_group[\u0027description\u0027],"},{"line_number":245,"context_line":"               \u0027default\u0027: bool(security_group.default_security_group)}"},{"line_number":246,"context_line":"        res[\u0027security_group_rules\u0027] \u003d [self._make_security_group_rule_dict(r)"},{"line_number":247,"context_line":"                                       for r in security_group.rules]"},{"line_number":248,"context_line":"        return self._fields(res, fields)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3a961159_1cbeef64","line":245,"in_reply_to":"3a961159_b6b5fa33","updated":"2015-01-20 17:06:19.000000000","message":"Yes, sorry, as per your comment it is not what we needed I was going to remove this at all, but forgot to do that.","commit_id":"beb602e21a253271d8d6df58796694a391da0dfa"}],"neutron/extensions/securitygroup.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"dd3442518d79770deb52e8086100408d5d542444","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"class DuplicateSecurityGroupsNamedDefault(qexception.Conflict):"},{"line_number":69,"context_line":"    message \u003d _(\"There are more than one security group with name \u0027default\u0027 \""},{"line_number":70,"context_line":"                \"in one tenant. Database scheme can\u0027t be upgraded.\")"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"class SecurityGroupRuleInvalidProtocol(qexception.InvalidInput):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_a9486a2f","line":69,"updated":"2014-12-25 13:32:23.000000000","message":"are \u003d\u003e is","commit_id":"8b4a0564d1542f3e685ecdb90a2d1402266a0253"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"c6ac570b334ace8244aedf27ed1c1eb1d66a1317","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"class DuplicateSecurityGroupsNamedDefault(qexception.Conflict):"},{"line_number":69,"context_line":"    message \u003d _(\"There are more than one security group with name \u0027default\u0027 \""},{"line_number":70,"context_line":"                \"in one tenant. Database scheme can\u0027t be upgraded.\")"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"class SecurityGroupRuleInvalidProtocol(qexception.InvalidInput):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_09017e06","line":69,"in_reply_to":"3a961159_a9486a2f","updated":"2014-12-25 14:33:14.000000000","message":"done","commit_id":"8b4a0564d1542f3e685ecdb90a2d1402266a0253"},{"author":{"_account_id":7962,"name":"Ronak Shah","email":"ronak.malav.shah@gmail.com","username":"ronak"},"change_message_id":"afb08b13d35862463d82d1f63e781ee0c0bbe130","unresolved":false,"context_lines":[{"line_number":68,"context_line":"class DuplicateSecurityGroupsNamedDefault(qexception.Conflict):"},{"line_number":69,"context_line":"    message \u003d _(\"There is more than one security group with name \u0027default\u0027 \""},{"line_number":70,"context_line":"                \"in one tenant. Database scheme can\u0027t be upgraded.\")"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"class SecurityGroupRuleInvalidProtocol(qexception.InvalidInput):"},{"line_number":74,"context_line":"    message \u003d _(\"Security group rule protocol %(protocol)s not supported. \""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_1dda0d4b","line":71,"updated":"2015-01-06 07:15:47.000000000","message":"Where is this being used?","commit_id":"c3f094ead6d12cfa7a11c178ecd0da935f2b0c93"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"4ac2287defe1cdcf730dd33e23b5a1dcfa0e8453","unresolved":false,"context_lines":[{"line_number":68,"context_line":"class DuplicateSecurityGroupsNamedDefault(qexception.Conflict):"},{"line_number":69,"context_line":"    message \u003d _(\"There is more than one security group with name \u0027default\u0027 \""},{"line_number":70,"context_line":"                \"in one tenant. Database scheme can\u0027t be upgraded.\")"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"class SecurityGroupRuleInvalidProtocol(qexception.InvalidInput):"},{"line_number":74,"context_line":"    message \u003d _(\"Security group rule protocol %(protocol)s not supported. \""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_f932275d","line":71,"in_reply_to":"3a961159_1dda0d4b","updated":"2015-01-12 11:00:37.000000000","message":"It is raised in migration.","commit_id":"c3f094ead6d12cfa7a11c178ecd0da935f2b0c93"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"96a4d892b7f4e5a0850bf20459db1cd2c343e490","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"class DuplicateSecurityGroupsNamedDefault(qexception.Conflict):"},{"line_number":69,"context_line":"    message \u003d _(\"Tenant %(tenant_id)s has security groups %(sg_ids)s with name\""},{"line_number":70,"context_line":"                \" \u0027default\u0027. Database scheme can\u0027t be upgraded. Should be only\""},{"line_number":71,"context_line":"                \" one security group with name \u0027default\u0027. \")"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_ffa54b03","line":69,"updated":"2015-01-14 14:06:32.000000000","message":"(No action required)\nIf I were you, I would move spaces to the ends of concatenated lines. But I\u0027m not, so you\u0027re free to leave it as-is. :)","commit_id":"1bbcd8695093a5482d0d97a746d447d56ba2e2eb"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"96a4d892b7f4e5a0850bf20459db1cd2c343e490","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"class DuplicateSecurityGroupsNamedDefault(qexception.Conflict):"},{"line_number":69,"context_line":"    message \u003d _(\"Tenant %(tenant_id)s has security groups %(sg_ids)s with name\""},{"line_number":70,"context_line":"                \" \u0027default\u0027. Database scheme can\u0027t be upgraded. Should be only\""},{"line_number":71,"context_line":"                \" one security group with name \u0027default\u0027. \")"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_ff6e2bc7","line":70,"updated":"2015-01-14 14:06:32.000000000","message":"Should be -\u003e There should be","commit_id":"1bbcd8695093a5482d0d97a746d447d56ba2e2eb"},{"author":{"_account_id":4656,"name":"Sean M. Collins","email":"sean@coreitpro.com","username":"scollins"},"change_message_id":"90bcbfcf73497070f7925a5f5a0d86fdd08d70b6","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"class DuplicateSecurityGroupsNamedDefault(qexception.Conflict):"},{"line_number":69,"context_line":"    message \u003d _(\"There are more than one security group with name \u0027default\u0027 in\""},{"line_number":70,"context_line":"                \" some tenants: %(duplications)s\")"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"class SecurityGroupRuleInvalidProtocol(qexception.InvalidInput):"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_48c93c59","line":70,"updated":"2015-01-14 20:06:13.000000000","message":"This class only is used in one place, in the migration script. Do you plan on using this in the future somewhere in the API? If not, can we just take this string and place it in the migration script and remove this class?","commit_id":"676a168dc9689d35c7cfeb9d1688144343beb6b0"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"fb904221c0df427e72396abe4a9681c6dc0ce551","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"class DuplicateSecurityGroupsNamedDefault(qexception.Conflict):"},{"line_number":69,"context_line":"    message \u003d _(\"There are more than one security group with name \u0027default\u0027 in\""},{"line_number":70,"context_line":"                \" some tenants: %(duplications)s\")"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"class SecurityGroupRuleInvalidProtocol(qexception.InvalidInput):"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_0f72c511","line":70,"in_reply_to":"3a961159_48c93c59","updated":"2015-01-15 11:02:50.000000000","message":"Just adding this string to error log in migration leads to really ugly logs, where important information placed somewhere in the middle http://paste.openstack.org/show/158088/. I will move this exception to migration.","commit_id":"676a168dc9689d35c7cfeb9d1688144343beb6b0"}],"neutron/tests/functional/db/test_migrations.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"5ea778617643b25a06760af1b3cae0503c7bc3d8","unresolved":false,"context_lines":[{"line_number":234,"context_line":"    pass"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"class TestSanityCheck(test_base.DbTestCase):"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    def setUp(self):"},{"line_number":240,"context_line":"        super(TestSanityCheck, self).setUp()"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_bb46cebd","line":237,"updated":"2015-01-28 18:48:16.000000000","message":"Nice work!","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a91912fca79513655b47daefff9be3104c785bb5","unresolved":false,"context_lines":[{"line_number":234,"context_line":"    pass"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"class TestSanityCheck(test_base.DbTestCase):"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    def setUp(self):"},{"line_number":240,"context_line":"        super(TestSanityCheck, self).setUp()"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_3dd1d257","line":237,"in_reply_to":"fa81d914_bb46cebd","updated":"2015-01-29 10:33:09.000000000","message":"thanks!","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"f5bc8ebd5d80c5c3057f32fb1ac3b00d6bc87c98","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            sqlalchemy.Column(\u0027name\u0027, sqlalchemy.String(255)),"},{"line_number":250,"context_line":"            sqlalchemy.Column(\u0027tenant_id\u0027, sqlalchemy.String(255)))"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        conn \u003d self.engine.connect()"},{"line_number":253,"context_line":"        SecurityGroup.create(self.engine)"},{"line_number":254,"context_line":"        conn.execute(SecurityGroup.insert(), ["},{"line_number":255,"context_line":"            {\u0027id\u0027: \u0027123d4s\u0027, \u0027tenant_id\u0027: \u0027sssda1\u0027, \u0027name\u0027: \u0027default\u0027},"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_ef795c94","line":252,"updated":"2015-01-28 19:49:58.000000000","message":"safer if this is \"with self.engine.connect() as conn:\"","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a91912fca79513655b47daefff9be3104c785bb5","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            sqlalchemy.Column(\u0027name\u0027, sqlalchemy.String(255)),"},{"line_number":250,"context_line":"            sqlalchemy.Column(\u0027tenant_id\u0027, sqlalchemy.String(255)))"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        conn \u003d self.engine.connect()"},{"line_number":253,"context_line":"        SecurityGroup.create(self.engine)"},{"line_number":254,"context_line":"        conn.execute(SecurityGroup.insert(), ["},{"line_number":255,"context_line":"            {\u0027id\u0027: \u0027123d4s\u0027, \u0027tenant_id\u0027: \u0027sssda1\u0027, \u0027name\u0027: \u0027default\u0027},"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_5dd4c646","line":252,"in_reply_to":"fa81d914_ef795c94","updated":"2015-01-29 10:33:09.000000000","message":"done","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"f5bc8ebd5d80c5c3057f32fb1ac3b00d6bc87c98","unresolved":false,"context_lines":[{"line_number":250,"context_line":"            sqlalchemy.Column(\u0027tenant_id\u0027, sqlalchemy.String(255)))"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        conn \u003d self.engine.connect()"},{"line_number":253,"context_line":"        SecurityGroup.create(self.engine)"},{"line_number":254,"context_line":"        conn.execute(SecurityGroup.insert(), ["},{"line_number":255,"context_line":"            {\u0027id\u0027: \u0027123d4s\u0027, \u0027tenant_id\u0027: \u0027sssda1\u0027, \u0027name\u0027: \u0027default\u0027},"},{"line_number":256,"context_line":"            {\u0027id\u0027: \u0027123d4\u0027, \u0027tenant_id\u0027: \u0027sssda1\u0027, \u0027name\u0027: \u0027default\u0027}"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_6fac8cee","line":253,"updated":"2015-01-28 19:49:58.000000000","message":"use \"conn\" here for the create.","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a91912fca79513655b47daefff9be3104c785bb5","unresolved":false,"context_lines":[{"line_number":250,"context_line":"            sqlalchemy.Column(\u0027tenant_id\u0027, sqlalchemy.String(255)))"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        conn \u003d self.engine.connect()"},{"line_number":253,"context_line":"        SecurityGroup.create(self.engine)"},{"line_number":254,"context_line":"        conn.execute(SecurityGroup.insert(), ["},{"line_number":255,"context_line":"            {\u0027id\u0027: \u0027123d4s\u0027, \u0027tenant_id\u0027: \u0027sssda1\u0027, \u0027name\u0027: \u0027default\u0027},"},{"line_number":256,"context_line":"            {\u0027id\u0027: \u0027123d4\u0027, \u0027tenant_id\u0027: \u0027sssda1\u0027, \u0027name\u0027: \u0027default\u0027}"}],"source_content_type":"text/x-python","patch_set":29,"id":"fa81d914_9dce3eb3","line":253,"in_reply_to":"fa81d914_6fac8cee","updated":"2015-01-29 10:33:09.000000000","message":"done","commit_id":"a8c5357eddc468d340a0748d6921d116c4d87bc5"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"28d20eed8d2e445ab6b06ebeb38cf9796009feea","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        self.alembic_config.neutron_config \u003d cfg.CONF"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    def test_check_sanity_14be42f3d0a5(self):"},{"line_number":245,"context_line":"        SecurityGroup \u003d sqlalchemy.Table("},{"line_number":246,"context_line":"            \u0027securitygroups\u0027, sqlalchemy.MetaData(),"},{"line_number":247,"context_line":"            sqlalchemy.Column(\u0027id\u0027, sqlalchemy.String(length\u003d36),"},{"line_number":248,"context_line":"                              nullable\u003dFalse),"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa81d914_6b1e0765","line":245,"updated":"2015-01-29 18:47:59.000000000","message":"Nit: SecurityGroup is just an instance so it should not be camelcase.","commit_id":"79c97120de9cff4d0992b5d41ff4bbf05e890f89"}],"neutron/tests/unit/test_db_migration.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c1332a23d009dd83a70bc5d43649544c7707bb68","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        self.mock_alembic_err.side_effect \u003d SystemExit"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def _main_test_helper(self, argv, func_name, exp_args\u003d(), exp_kwargs\u003d{}):"},{"line_number":79,"context_line":"        with mock.patch.object(sys, \u0027argv\u0027, argv), mock.patch.object("},{"line_number":80,"context_line":"                cli, \u0027run_sanity_checks\u0027):"},{"line_number":81,"context_line":"            cli.main()"},{"line_number":82,"context_line":"            self.do_alembic_cmd.assert_has_calls("}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_4fc0a82a","line":79,"updated":"2015-01-22 15:00:08.000000000","message":"It doesn\u0027t look especially nice. How about contextlib.nested?","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5f20c5c43bc76997233867ace891adc9e8921dc6","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        self.mock_alembic_err.side_effect \u003d SystemExit"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def _main_test_helper(self, argv, func_name, exp_args\u003d(), exp_kwargs\u003d{}):"},{"line_number":79,"context_line":"        with mock.patch.object(sys, \u0027argv\u0027, argv), mock.patch.object("},{"line_number":80,"context_line":"                cli, \u0027run_sanity_checks\u0027):"},{"line_number":81,"context_line":"            cli.main()"},{"line_number":82,"context_line":"            self.do_alembic_cmd.assert_has_calls("}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_9030d102","line":79,"in_reply_to":"1a930d6b_4fc0a82a","updated":"2015-01-23 11:23:53.000000000","message":"Yes, I agree, that it does not look pretty, but  I don\u0027t think that using deprecated method is a good idea.","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"2a25b7f324bed84f85a05d8b71f82d23fc4a55c1","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        self.mock_alembic_err.side_effect \u003d SystemExit"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def _main_test_helper(self, argv, func_name, exp_args\u003d(), exp_kwargs\u003d{}):"},{"line_number":79,"context_line":"        with mock.patch.object(sys, \u0027argv\u0027, argv), mock.patch.object("},{"line_number":80,"context_line":"                cli, \u0027run_sanity_checks\u0027):"},{"line_number":81,"context_line":"            cli.main()"},{"line_number":82,"context_line":"            self.do_alembic_cmd.assert_has_calls("}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_ff647e4a","line":79,"in_reply_to":"1a930d6b_9030d102","updated":"2015-01-23 11:40:31.000000000","message":"I think question on how we migrate from contextlib.nested is another story that should be solved separately. AFAIK Python3 still does not provide clear and easy alternative to it.","commit_id":"9e68105aa3a1b4d18e2d4cd138cbdbea69e8c08a"}]}