)]}'
{"neutron/plugins/midonet/midonet_lib.py":[{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e0e9d9ee8a5ae406b780c11c3447489d37e99f8d","unresolved":false,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        for r in chain.get_rules():"},{"line_number":369,"context_line":"            props \u003d r.get_properties()"},{"line_number":370,"context_line":"            if props is not None and key in props and props[key] \u003d\u003d value:"},{"line_number":371,"context_line":"                self.mido_api.delete_rule(r.get_id())"},{"line_number":372,"context_line":""},{"line_number":373,"context_line":"    @handle_api_error"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FZ8Q%3D","line":370,"updated":"2013-09-27 11:29:51.000000000","message":"The test \"props is not None\" is redundant. If the \"key in props\" clause is true, then you won\u0027t have a problem accessing props[key].","commit_id":"d4ed24c6502137608094776ef4e07bd41af2303b"},{"author":{"_account_id":9483,"name":"LiShaokai","email":"lshaokai@cn.ibm.com","username":"lshaokai"},"change_message_id":"e1b892af915244dbe7221793b6595231c706729f","unresolved":false,"context_lines":[{"line_number":413,"context_line":""},{"line_number":414,"context_line":"        for r in chain.get_rules():"},{"line_number":415,"context_line":"            props \u003d r.get_properties()"},{"line_number":416,"context_line":"            if props is not None and key in props and props[key] \u003d\u003d value:"},{"line_number":417,"context_line":"                self.mido_api.delete_rule(r.get_id())"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    @handle_api_error"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F9qhA%3D","line":416,"updated":"2013-12-09 04:00:15.000000000","message":"NIT: if \"value\" is always not NULL, you can use \n\nif props is not None and props.get(key) \u003d\u003d value:","commit_id":"1311db964cc49bc4cc44e6c8502dd383a71c6c38"}],"neutron/plugins/midonet/plugin.py":[{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e0e9d9ee8a5ae406b780c11c3447489d37e99f8d","unresolved":false,"context_lines":[{"line_number":246,"context_line":""},{"line_number":247,"context_line":"    def _initialize_port_chains(self, context, port,"},{"line_number":248,"context_line":"                                in_chain, out_chain, sg_ids):"},{"line_number":249,"context_line":"        \"\"\" Initializes ingress and egress security rule chains for"},{"line_number":250,"context_line":"        the specified port."},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        in_chain - Empty chain to be initialized. This is the chain"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FZ70%3D","line":249,"updated":"2013-09-27 11:29:51.000000000","message":"Nit: PEP257 put summary docstring on one line, and rest on additional lines.","commit_id":"d4ed24c6502137608094776ef4e07bd41af2303b"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e0e9d9ee8a5ae406b780c11c3447489d37e99f8d","unresolved":false,"context_lines":[{"line_number":272,"context_line":"        # Both chains drop non-ARP traffic if no other rules match."},{"line_number":273,"context_line":"        for chain in [in_chain, out_chain]:"},{"line_number":274,"context_line":"            self._add_chain_rule(chain, action\u003d\u0027drop\u0027,"},{"line_number":275,"context_line":"                                 dl_type\u003d0x0806,  # ARP"},{"line_number":276,"context_line":"                                 inv_dl_type\u003dTrue, position\u003d1)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        # Add reverse flow matching for in_chain."}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FZ7s%3D","line":275,"updated":"2013-09-27 11:29:51.000000000","message":"Is there a constant that can be used, instead of a magic # for ARP?","commit_id":"d4ed24c6502137608094776ef4e07bd41af2303b"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e0e9d9ee8a5ae406b780c11c3447489d37e99f8d","unresolved":false,"context_lines":[{"line_number":288,"context_line":"        # IP spoofing protection for in_chain"},{"line_number":289,"context_line":"        for fixed_ip in port[\"fixed_ips\"]:"},{"line_number":290,"context_line":"            subnet \u003d self._get_subnet(context, fixed_ip[\"subnet_id\"])"},{"line_number":291,"context_line":"            ipv6 \u003d (subnet[\"ip_version\"] \u003d\u003d 6)"},{"line_number":292,"context_line":"            dl_type, mask \u003d (0x86dd, \"/128\") if ipv6 else (0x0800, \"/32\")"},{"line_number":293,"context_line":"            self._add_chain_rule(in_chain, action\u003d\"drop\","},{"line_number":294,"context_line":"                                 src_addr\u003dfixed_ip[\"ip_address\"] + mask,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FZ64%3D","line":291,"updated":"2013-09-27 11:29:51.000000000","message":"ipv6 is only used on next line. Why not just say:\n\nif subnet[\"ip_verion\"] \u003d\u003d 6:\n    ...\nelse:\n    ...\n\nMore magic numbers here. Would be good to have constants.","commit_id":"d4ed24c6502137608094776ef4e07bd41af2303b"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e0e9d9ee8a5ae406b780c11c3447489d37e99f8d","unresolved":false,"context_lines":[{"line_number":309,"context_line":""},{"line_number":310,"context_line":"    def _add_sg_jumps_to_port_chains(self, port_inbound, port_outbound,"},{"line_number":311,"context_line":"                                     sg_egress, sg_ingress):"},{"line_number":312,"context_line":"        \"\"\" Add rules to jump from port_inbound to sg_egress and from"},{"line_number":313,"context_line":"        port_outbound to sg_ingress at the appropriate place in each"},{"line_number":314,"context_line":"        port chain. Port_inbound and sg_egress are the port and"},{"line_number":315,"context_line":"        security group chains for traffic out of a VM and into the"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FZ6M%3D","line":312,"updated":"2013-09-27 11:29:51.000000000","message":"Nit: PEP257 put one line summary sentence, blank line and then rest of docstring.","commit_id":"d4ed24c6502137608094776ef4e07bd41af2303b"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e0e9d9ee8a5ae406b780c11c3447489d37e99f8d","unresolved":false,"context_lines":[{"line_number":316,"context_line":"        network, while port_outbound and sg_ingress are for traffic out"},{"line_number":317,"context_line":"        of the network and into a VM. \"\"\""},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        # Insert jumps into 4th position for inbound chain and 2nd"},{"line_number":320,"context_line":"        # position for outbound chain. Each chain has a rule to match"},{"line_number":321,"context_line":"        # reverse flow which must precede the SG jump rules, and the"},{"line_number":322,"context_line":"        # inbound chain also has two anti-spoofing rules."}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FZ6I%3D","line":319,"updated":"2013-09-27 11:29:51.000000000","message":"Make part of docstring?","commit_id":"d4ed24c6502137608094776ef4e07bd41af2303b"}],"neutron/tests/unit/midonet/mock_lib.py":[{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"8d0e7fb0399a07d4cdf8f43a49c2cfefab75009d","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    r.get_tenant_id.return_value \u003d tenant_id"},{"line_number":77,"context_line":"    r.get_name.return_value \u003d name"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    for k, v in kwargs.iteritems():"},{"line_number":80,"context_line":"        if k \u003d\u003d \"dl_src\":"},{"line_number":81,"context_line":"            r.get_dl_src.return_value \u003d v"},{"line_number":82,"context_line":"        elif k \u003d\u003d \"dl_type\":"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2BWEQ%3D","line":79,"updated":"2013-11-27 05:19:20.000000000","message":"Instead of a conditional of 20 clause consider:\n\nattrs \u003d [\u003clist of attrs]\u003e\nfor k, v in kwargs.iteritems():\n    if k in attrs:\n        getattr(r, k).return_value \u003d v","commit_id":"98941d4935ccf19838b873ce78a63afc1851f3f3"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"8301fa5bcc4f0824921623f26a468fab68f21955","unresolved":false,"context_lines":[{"line_number":239,"context_line":"        for chain in self.chains.values():"},{"line_number":240,"context_line":"            if chain.get_name() \u003d\u003d name and chain.get_tenant_id() \u003d\u003d tenant_id:"},{"line_number":241,"context_line":"                return chain"},{"line_number":242,"context_line":"        return None"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    def _add_chain_rule(self, chain, action, query\u003dNone, **kwargs):"},{"line_number":245,"context_line":"        tenant_id \u003d self._get_query_tenant_id(query)"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAUH%2F%2F8l4%3D","line":242,"updated":"2014-01-06 11:45:11.000000000","message":"Not needed, as it is an implied return value.","commit_id":"3a0ffa3a475cc5ae182b98fb75f8ae110eac9dcb"}],"neutron/tests/unit/midonet/test_midonet_plugin.py":[{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"49a33809879c1b0eac6c8cfb97fffa09116f344e","unresolved":false,"context_lines":[{"line_number":149,"context_line":"                raise Exception(\"Unexpected chain: \" + chain.get_name())"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            for rule in chain.get_rules():"},{"line_number":152,"context_line":"                assert rule.get_chain_id() \u003d\u003d chain.get_id()"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    def _verify_inbound_chain(self, chain):"},{"line_number":155,"context_line":"        rules \u003d chain.get_rules()"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATH%2F%2F9t0%3D","line":152,"updated":"2013-10-07 12:29:43.000000000","message":"All the asserts in the test cases, should be self.assert*() and not just assert.  For example, this line would be:\n\nself.assertEquals(rule.get_chain_id(), chain.get_id())\n\nSome of them below, can use self.assertTrue(), self.assertFalse(), and self.assertIsNotNone().","commit_id":"0730f38015b4779d37b697ded55fb183a83a5ee3"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"8fe4256f45d8ea1000c1bce4c96d437ace3ba0f7","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_create_port_json(self):"},{"line_number":137,"context_line":"        super(TestMidonetPortsV2, self).test_create_port_json()"},{"line_number":138,"context_line":"        chains \u003d self.instance().get_chains(query\u003dNone)"},{"line_number":139,"context_line":"        for chain in chains:"},{"line_number":140,"context_line":"            if inbound_chain_regex.match(chain.get_name()):"},{"line_number":141,"context_line":"                self._verify_inbound_chain(chain)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATH%2F%2F5QM%3D","line":138,"updated":"2013-10-08 12:03:11.000000000","message":"It would be clearer (to me) to have separate test cases for each of the chains, rather than one looping test case w/conditions to call _verify_*() functions.","commit_id":"62ce85ba637627b801f7cae0143f4467dd8d0bee"},{"author":{"_account_id":8858,"name":"Brandon Berg","email":"bberg@midokura.jp","username":"bberg"},"change_message_id":"1d3c8495732f9cee3604ebf1b2a4ae29b9f1627e","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_create_port_json(self):"},{"line_number":137,"context_line":"        super(TestMidonetPortsV2, self).test_create_port_json()"},{"line_number":138,"context_line":"        chains \u003d self.instance().get_chains(query\u003dNone)"},{"line_number":139,"context_line":"        for chain in chains:"},{"line_number":140,"context_line":"            if inbound_chain_regex.match(chain.get_name()):"},{"line_number":141,"context_line":"                self._verify_inbound_chain(chain)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATH%2F%2F1xs%3D","line":138,"in_reply_to":"AAAATH%2F%2F5QM%3D","updated":"2013-10-09 05:50:09.000000000","message":"I\u0027m testing that a new port will have the desired rule chains. I could test the function that initializes the rule chains directly, but it would leave some coverage holes that are closed by doing it this way. I added some comments to explain why I loop through the chains and where to look if this test fails.","commit_id":"62ce85ba637627b801f7cae0143f4467dd8d0bee"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"8fe4256f45d8ea1000c1bce4c96d437ace3ba0f7","unresolved":false,"context_lines":[{"line_number":146,"context_line":"            elif ingress_chain_regex.match(chain.get_name()):"},{"line_number":147,"context_line":"                self._verify_ingress_chain(chain)"},{"line_number":148,"context_line":"            else:"},{"line_number":149,"context_line":"                raise Exception(\"Unexpected chain: \" + chain.get_name())"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            for rule in chain.get_rules():"},{"line_number":152,"context_line":"                self.assertEqual(chain.get_id(), rule.get_chain_id())"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATH%2F%2F5QI%3D","line":149,"updated":"2013-10-08 12:03:11.000000000","message":"this could be self.fail(\"Msg\")","commit_id":"62ce85ba637627b801f7cae0143f4467dd8d0bee"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"8fe4256f45d8ea1000c1bce4c96d437ace3ba0f7","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        # IP spoofing prevention."},{"line_number":158,"context_line":"        self.assertEqual(ETHERTYPE_IP4, rules[0].get_dl_type())"},{"line_number":159,"context_line":"        self.assertEqual(\"drop\", rules[0].get_flow_action())"},{"line_number":160,"context_line":"        self.assertEqual(True, rules[0].get_inv_nw_src())"},{"line_number":161,"context_line":"        self.assertEqual(\"10.0.0.2\", rules[0].get_nw_src_address())"},{"line_number":162,"context_line":"        self.assertEqual(32, rules[0].get_nw_src_length())"},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATH%2F%2F5V0%3D","line":160,"updated":"2013-10-08 12:03:11.000000000","message":"You can use assertTrue() and assertFalse() functions.","commit_id":"62ce85ba637627b801f7cae0143f4467dd8d0bee"},{"author":{"_account_id":8858,"name":"Brandon Berg","email":"bberg@midokura.jp","username":"bberg"},"change_message_id":"1d3c8495732f9cee3604ebf1b2a4ae29b9f1627e","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        # IP spoofing prevention."},{"line_number":158,"context_line":"        self.assertEqual(ETHERTYPE_IP4, rules[0].get_dl_type())"},{"line_number":159,"context_line":"        self.assertEqual(\"drop\", rules[0].get_flow_action())"},{"line_number":160,"context_line":"        self.assertEqual(True, rules[0].get_inv_nw_src())"},{"line_number":161,"context_line":"        self.assertEqual(\"10.0.0.2\", rules[0].get_nw_src_address())"},{"line_number":162,"context_line":"        self.assertEqual(32, rules[0].get_nw_src_length())"},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATH%2F%2F1x8%3D","line":160,"in_reply_to":"AAAATH%2F%2F5V0%3D","updated":"2013-10-09 05:50:09.000000000","message":"Whoops. I did a regex search/replace to convert assert x \u003d\u003d y to self.assertEqual(y, x) but forgot that that would catch the True/False cases, too.","commit_id":"62ce85ba637627b801f7cae0143f4467dd8d0bee"}]}