)]}'
{"neutron/db/migration/alembic_migrations/versions/39cf3f799352_fwaas_havana_2_model.py":[{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        sa.Column(\u0027name\u0027, sa.String(length\u003d255), nullable\u003dTrue),"},{"line_number":83,"context_line":"        sa.Column(\u0027description\u0027, sa.String(length\u003d1024), nullable\u003dTrue),"},{"line_number":84,"context_line":"        sa.Column(\u0027firewall_policy_id\u0027, sa.String(length\u003d36), nullable\u003dTrue),"},{"line_number":85,"context_line":"        sa.Column(\u0027service_group_id\u0027, sa.String(length\u003d36), nullable\u003dTrue),"},{"line_number":86,"context_line":"        sa.Column(\u0027shared\u0027, sa.Boolean(), autoincrement\u003dFalse,"},{"line_number":87,"context_line":"                  nullable\u003dTrue),"},{"line_number":88,"context_line":"        sa.Column(\u0027protocol\u0027, sa.String(length\u003d24), nullable\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_1d49418b","line":85,"updated":"2014-07-03 05:19:10.000000000","message":"that needs to be in a new migration, not the fix of the old one","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"}],"neutron/db/servicegroup_db.py":[{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"class ServiceGroup(model_base.BASEV2, models_v2.HasId, models_v2.HasTenant):"},{"line_number":56,"context_line":"    name \u003d sa.Column(sa.String(255))"},{"line_number":57,"context_line":"    description \u003d sa.Column(sa.String(255))"},{"line_number":58,"context_line":"    service_objects \u003d orm.relationship("},{"line_number":59,"context_line":"        ServiceObject,"},{"line_number":60,"context_line":"        backref\u003dorm.backref(\u0027servicegroups\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_3d2605cf","line":57,"updated":"2014-07-03 05:19:10.000000000","message":"nit: description is usually 1024","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        return self._create_bulk(\u0027servicegroups\u0027, context, service_object)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def create_service_group(self, context, service_group):"},{"line_number":73,"context_line":"        LOG.debug(_(\"ServiceGroup create service group is called\"))"},{"line_number":74,"context_line":"        s \u003d service_group[\u0027service_group\u0027]"},{"line_number":75,"context_line":"        tenant_id \u003d self._get_tenant_id_for_create(context, s)"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_5d08192d","line":73,"updated":"2014-07-03 05:19:10.000000000","message":"nit: per new policy, i18n for debug messages is not needed","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def _get_service_group(self, context, sgid):"},{"line_number":108,"context_line":"        query \u003d self._model_query(context, ServiceGroup)"},{"line_number":109,"context_line":"        return query.filter_by(id\u003dsgid).one()"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def delete_service_group(self, context, sgid):"},{"line_number":112,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_285c0525","line":109,"updated":"2014-07-03 05:19:10.000000000","message":"usually _get_object() checks for NoResultFound and throws object-specific NotFound exception.\n\nOtherwise querying non-existing service group will lead to 500 error from the server","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":114,"context_line":"                sg \u003d self._get_service_group(context, sgid)"},{"line_number":115,"context_line":"                context.session.delete(sg)"},{"line_number":116,"context_line":"        except sqlexc.IntegrityError:"},{"line_number":117,"context_line":"            raise servg.ServiceGroupInUse(id\u003dsgid)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def update_service_group(self, context, sgid, service_group):"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_a88c9580","line":116,"updated":"2014-07-03 05:19:10.000000000","message":"i\u0027d suggest to check if service group is in use explicitly, rather than catching sql exception","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":216,"context_line":"            tenant_ids.add(sobject[\u0027tenant_id\u0027])"},{"line_number":217,"context_line":"            new_objects.add(sobject[\u0027service_group_id\u0027])"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        if len(new_objects) \u003e 1:"},{"line_number":220,"context_line":"            raise servg.ServiceGroupNotSingleGroup()"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        if len(tenant_ids) \u003e 1:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_e8620dac","line":219,"updated":"2014-07-03 05:19:10.000000000","message":"i think this and the check below should be moved in the beginning of the method","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def _get_min_max_ports_from_range(self, port_range):"},{"line_number":230,"context_line":"        if not port_range:"},{"line_number":231,"context_line":"            return [None, None]"},{"line_number":232,"context_line":"        min_port, sep, max_port \u003d port_range.partition(\":\")"},{"line_number":233,"context_line":"        if not max_port:"},{"line_number":234,"context_line":"            max_port \u003d min_port"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_686efd97","line":231,"updated":"2014-07-03 05:19:10.000000000","message":"nit: a tuple should be returned instead of a list","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":269,"context_line":"                res[key] \u003d [value]"},{"line_number":270,"context_line":"        return res"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"    def _check_for_duplicate_objects(self, context, service_objects):"},{"line_number":273,"context_line":"        for i in service_objects:"},{"line_number":274,"context_line":"            self_found \u003d False"},{"line_number":275,"context_line":"            for j in service_objects:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_68e35d05","line":272,"updated":"2014-07-03 05:19:10.000000000","message":"suggestion: _check_for_duplicate_service_objects","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":270,"context_line":"        return res"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"    def _check_for_duplicate_objects(self, context, service_objects):"},{"line_number":273,"context_line":"        for i in service_objects:"},{"line_number":274,"context_line":"            self_found \u003d False"},{"line_number":275,"context_line":"            for j in service_objects:"},{"line_number":276,"context_line":"                if i[\u0027service_object\u0027] \u003d\u003d j[\u0027service_object\u0027]:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_e8a5adb5","line":273,"updated":"2014-07-03 05:19:10.000000000","message":"nit: please more descriptive name. \u0027svc_obj\u0027?","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    def _get_service_object(self, context, obj_id):"},{"line_number":308,"context_line":"        query \u003d self._model_query(context, ServiceObject)"},{"line_number":309,"context_line":"        return query.filter_by(id\u003dobj_id).one()"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"    def update_service_object(self, context, soid, service_object):"},{"line_number":312,"context_line":"        s \u003d service_object[\u0027service_object\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_487db91b","line":309,"updated":"2014-07-03 05:19:10.000000000","message":"you should catch NoResultFound and throw another exception.","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":312,"context_line":"        s \u003d service_object[\u0027service_object\u0027]"},{"line_number":313,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":314,"context_line":"            so \u003d self._get_service_object(context, soid)"},{"line_number":315,"context_line":"            if so is None:"},{"line_number":316,"context_line":"                raise servg.ServiceObjectNotFound(id\u003dsoid)"},{"line_number":317,"context_line":"            so.update(s)"},{"line_number":318,"context_line":"        return self._make_service_object_dict(so)"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_88aef183","line":315,"updated":"2014-07-03 05:19:10.000000000","message":"None is never returned in this case. If the comment above is fixed, you would not need to do this check at all.","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"}],"neutron/extensions/servicegroup.py":[{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        if data.lower() is \"any\":"},{"line_number":136,"context_line":"            return"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    msg \u003d _(\"Invalid icmp code \u0027%s\u0027\") % data"},{"line_number":139,"context_line":"    LOG.debug(msg)"},{"line_number":140,"context_line":"    return msg"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_c861c93a","line":138,"updated":"2014-07-03 05:19:10.000000000","message":"Invalid icmp type? The method seems identical to previous one, could they be refactored?","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"}],"neutron/services/firewall/agents/l3reference/firewall_l3_agent.py":[{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    def get_service_groups_for_firewall_rules(self, context,"},{"line_number":60,"context_line":"                                              service_group_ids):"},{"line_number":61,"context_line":"        \"\"\" Get service groups for firewall rules from plugin.\"\"\""},{"line_number":62,"context_line":"        LOG.debug(_(\"Retrive service group from plugin\"))"},{"line_number":63,"context_line":"        return self.call(context,"},{"line_number":64,"context_line":"                         self.make_msg(\u0027get_service_groups_for_firewall_rules\u0027,"},{"line_number":65,"context_line":"                                       host\u003dself.host,"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_48d21901","line":62,"updated":"2014-07-03 05:19:10.000000000","message":"nit: no i18n for debug messages","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"}],"neutron/tests/unit/test_extension_service_group.py":[{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# vim: tabstop\u003d4 shiftwidth\u003d4 softtabstop\u003d4"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"# Copyright 2013 vArmour Networks Inc."},{"line_number":4,"context_line":"# All Rights Reserved."}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_288885e6","line":1,"updated":"2014-07-03 05:19:10.000000000","message":"please drop this line","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# vim: tabstop\u003d4 shiftwidth\u003d4 softtabstop\u003d4"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"# Copyright 2013 vArmour Networks Inc."},{"line_number":4,"context_line":"# All Rights Reserved."},{"line_number":5,"context_line":"#"},{"line_number":6,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_a81c95c2","line":3,"updated":"2014-07-03 05:19:10.000000000","message":"2014?","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    def _make_service_group(self, fmt, name, description, **kwargs):"},{"line_number":72,"context_line":"        res \u003d self._create_service_group(fmt, name, description, **kwargs)"},{"line_number":73,"context_line":"        if res.status_int \u003e\u003d exc.HTTPBadRequest.code:"},{"line_number":74,"context_line":"            raise exc.HTTPClientError(code\u003dres.status_int)"},{"line_number":75,"context_line":"        return self.deserialize(fmt, res)"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    @contextlib.contextmanager"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_e82f6df4","line":74,"updated":"2014-07-03 05:19:10.000000000","message":"why raise and not assert?\nApplies to some code below as well.","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":277,"context_line":"                                          timeout\u003d0)"},{"line_number":278,"context_line":"        res \u003d self._create_service_object(self.fmt, data)"},{"line_number":279,"context_line":"        self.deserialize(self.fmt, res)"},{"line_number":280,"context_line":"        self.assertEqual(res.status_int, exc.HTTPInternalServerError.code)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"    def test_get_service_group(self):"},{"line_number":283,"context_line":"        name \u003d \u0027testservicegroup\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_281165ac","line":280,"updated":"2014-07-03 05:19:10.000000000","message":"status code check should be before deserialization","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":319,"context_line":"                                              timeout\u003dtm)"},{"line_number":320,"context_line":"            res \u003d self._create_service_object(self.fmt, data)"},{"line_number":321,"context_line":"            self.deserialize(self.fmt, res)"},{"line_number":322,"context_line":"            self.assertEqual(res.status_int, exc.HTTPBadRequest.code)"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"    def test_create_service_object_without_protocol(self):"},{"line_number":325,"context_line":"        self._service_object_negative_test_case(sport\u003d\u002710:100\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_68227d02","line":322,"updated":"2014-07-03 05:19:10.000000000","message":"same as above","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8f8b67d30714a1916f5d434f07e65c907d98e0ae","unresolved":false,"context_lines":[{"line_number":545,"context_line":"                             exc.HTTPInternalServerError.code)"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"class TestServiceGroupXML(TestServiceGroup):"},{"line_number":549,"context_line":"    fmt \u003d \u0027xml\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_e8084d93","line":548,"updated":"2014-07-03 05:19:10.000000000","message":"I think neutron tend to move away from supporting XML, so I\u0027m not sure we want to keep another version of unit tests","commit_id":"88cdc04682eac964374000e95fa884df8ddee0d2"}]}