)]}'
{"doc/source/devref/openvswitch_firewall.rst":[{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"407e5da138c324f0c7aae840198cb6b29564ff0c","unresolved":false,"context_lines":[{"line_number":100,"context_line":"m are number of ports in the remote group ID and the port security group,"},{"line_number":101,"context_line":"respectively."},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"A conj_id is allocated for each (remote_group_id, security_group_id,"},{"line_number":104,"context_line":"direction, ethertype) tuple.  The class ``ConjIdMap`` handles the"},{"line_number":105,"context_line":"mapping. The same conj_id is shared between security group rules if"},{"line_number":106,"context_line":"multiple rules belong to the same tuple above."}],"source_content_type":"text/x-rst","patch_set":15,"id":"7a77a97e_b16c59d1","line":103,"updated":"2016-11-17 13:55:20.000000000","message":"Have you considered adding tenant information? Since you are identifying ports by IPs, could there be IP collision across tenants?","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"fadbc5525ca8ee3f9bb24fbdea49fb6bc5482085","unresolved":false,"context_lines":[{"line_number":100,"context_line":"m are number of ports in the remote group ID and the port security group,"},{"line_number":101,"context_line":"respectively."},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"A conj_id is allocated for each (remote_group_id, security_group_id,"},{"line_number":104,"context_line":"direction, ethertype) tuple.  The class ``ConjIdMap`` handles the"},{"line_number":105,"context_line":"mapping. The same conj_id is shared between security group rules if"},{"line_number":106,"context_line":"multiple rules belong to the same tuple above."}],"source_content_type":"text/x-rst","patch_set":15,"id":"7a77a97e_b8f7bc57","line":103,"in_reply_to":"7a77a97e_b16c59d1","updated":"2016-11-18 06:38:50.000000000","message":"Isn\u0027t tenant isolation implied by security group IDs?","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"407e5da138c324f0c7aae840198cb6b29564ff0c","unresolved":false,"context_lines":[{"line_number":317,"context_line":"::"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":" table\u003d82, priority\u003d70,ct_state\u003d+est-rel-rpl,icmp,reg5\u003d0x2,dl_dst\u003dfa:16:3e:24:57:c7,nw_src\u003d192.168.0.1 actions\u003dstrip_vlan,output:2"},{"line_number":320,"context_line":" table\u003d82, priority\u003d70,ct_state\u003d+new-est,icmp,reg5\u003d0x2,dl_dst\u003dfa:16:3e:24:57:c7,nw_src\u003d192.168.0.1 actions\u003dct(commit,zone\u003dNXM_NX_REG6[0..15]),strip_vlan,output:2"},{"line_number":321,"context_line":" table\u003d82, priority\u003d50,ct_state\u003d+inv+trk actions\u003ddrop"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"The mechanism for dropping connections that are not allowed anymore is the"}],"source_content_type":"text/x-rst","patch_set":15,"id":"7a77a97e_1ef99cc7","line":320,"updated":"2016-11-17 13:55:20.000000000","message":"If I understand correctly, the flows in this example will be modified to use conjunction. Perhaps update the example?","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"fadbc5525ca8ee3f9bb24fbdea49fb6bc5482085","unresolved":false,"context_lines":[{"line_number":317,"context_line":"::"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":" table\u003d82, priority\u003d70,ct_state\u003d+est-rel-rpl,icmp,reg5\u003d0x2,dl_dst\u003dfa:16:3e:24:57:c7,nw_src\u003d192.168.0.1 actions\u003dstrip_vlan,output:2"},{"line_number":320,"context_line":" table\u003d82, priority\u003d70,ct_state\u003d+new-est,icmp,reg5\u003d0x2,dl_dst\u003dfa:16:3e:24:57:c7,nw_src\u003d192.168.0.1 actions\u003dct(commit,zone\u003dNXM_NX_REG6[0..15]),strip_vlan,output:2"},{"line_number":321,"context_line":" table\u003d82, priority\u003d50,ct_state\u003d+inv+trk actions\u003ddrop"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"The mechanism for dropping connections that are not allowed anymore is the"}],"source_content_type":"text/x-rst","patch_set":15,"id":"7a77a97e_780b642a","line":320,"in_reply_to":"7a77a97e_1ef99cc7","updated":"2016-11-18 06:38:50.000000000","message":"Good catch. Done.","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"081ca4b854a785f3c6d3fe65f48e93c3e61ec72e","unresolved":false,"context_lines":[{"line_number":96,"context_line":"With a security group rule with a remote group ID, flows that match on"},{"line_number":97,"context_line":"nw_src for remote_group_id addresses and match on dl_dst for port MAC"},{"line_number":98,"context_line":"addresses are needed (for ingress rules; likewise for egress"},{"line_number":99,"context_line":"rules). Without conjunction, this results in O(n*m) flows where n and"},{"line_number":100,"context_line":"m are number of ports in the remote group ID and the port security group,"},{"line_number":101,"context_line":"respectively."},{"line_number":102,"context_line":""}],"source_content_type":"text/x-rst","patch_set":21,"id":"ba5201f7_97f7ead1","line":99,"updated":"2017-01-06 23:27:28.000000000","message":"Big O can be used to describe the space used by an algorithm.","commit_id":"fcecc59de2b7fae0daa02d19ec3ee4640beaa1f2"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"1c207a1205c3c5fa0c90c12cfd95d467b4780fe1","unresolved":false,"context_lines":[{"line_number":96,"context_line":"With a security group rule with a remote group ID, flows that match on"},{"line_number":97,"context_line":"nw_src for remote_group_id addresses and match on dl_dst for port MAC"},{"line_number":98,"context_line":"addresses are needed (for ingress rules; likewise for egress"},{"line_number":99,"context_line":"rules). Without conjunction, this results in O(n*m) flows where n and"},{"line_number":100,"context_line":"m are number of ports in the remote group ID and the port security group,"},{"line_number":101,"context_line":"respectively."},{"line_number":102,"context_line":""}],"source_content_type":"text/x-rst","patch_set":21,"id":"ba5201f7_7c7f0dab","line":99,"range":{"start_line":99,"start_character":45,"end_line":99,"end_character":51},"updated":"2017-01-05 15:30:13.000000000","message":"There should be just \"n*m\", number of flows is not a function, is it? :)","commit_id":"fcecc59de2b7fae0daa02d19ec3ee4640beaa1f2"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e668f9481609537627b3859e1295b817d8dbcbfc","unresolved":false,"context_lines":[{"line_number":96,"context_line":"With a security group rule with a remote group ID, flows that match on"},{"line_number":97,"context_line":"nw_src for remote_group_id addresses and match on dl_dst for port MAC"},{"line_number":98,"context_line":"addresses are needed (for ingress rules; likewise for egress"},{"line_number":99,"context_line":"rules). Without conjunction, this results in O(n*m) flows where n and"},{"line_number":100,"context_line":"m are number of ports in the remote group ID and the port security group,"},{"line_number":101,"context_line":"respectively."},{"line_number":102,"context_line":""}],"source_content_type":"text/x-rst","patch_set":21,"id":"ba5201f7_6d857558","line":99,"range":{"start_line":99,"start_character":45,"end_line":99,"end_character":51},"in_reply_to":"ba5201f7_09da284c","updated":"2017-01-06 10:16:49.000000000","message":"You\u0027re talking about count of flows without using conjunctions and not about algorithm complexity. Big O is to analyse algorithm complexity.","commit_id":"fcecc59de2b7fae0daa02d19ec3ee4640beaa1f2"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"bb3ac8d53db444b95f268a225012a2bed7baeb06","unresolved":false,"context_lines":[{"line_number":96,"context_line":"With a security group rule with a remote group ID, flows that match on"},{"line_number":97,"context_line":"nw_src for remote_group_id addresses and match on dl_dst for port MAC"},{"line_number":98,"context_line":"addresses are needed (for ingress rules; likewise for egress"},{"line_number":99,"context_line":"rules). Without conjunction, this results in O(n*m) flows where n and"},{"line_number":100,"context_line":"m are number of ports in the remote group ID and the port security group,"},{"line_number":101,"context_line":"respectively."},{"line_number":102,"context_line":""}],"source_content_type":"text/x-rst","patch_set":21,"id":"ba5201f7_09da284c","line":99,"range":{"start_line":99,"start_character":45,"end_line":99,"end_character":51},"in_reply_to":"ba5201f7_7c7f0dab","updated":"2017-01-06 04:43:56.000000000","message":"Precisely it is 2*n*m and Big O notation is valid here as we aren\u0027t interested in a constant coefficient (2).\n\nhttps://en.wikipedia.org/wiki/Analysis_of_algorithms#Orders_of_growth","commit_id":"fcecc59de2b7fae0daa02d19ec3ee4640beaa1f2"}],"neutron/agent/linux/openvswitch_firewall/firewall.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"fe59f69f1dab60bbd07cf6a69f5b54b226d52de1","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        self.int_br \u003d self.initialize_bridge(integration_bridge)"},{"line_number":187,"context_line":"        self.sg_port_map \u003d SGPortMap()"},{"line_number":188,"context_line":"        self._deferred \u003d False"},{"line_number":189,"context_line":"        self.conj_id_map \u003d dict()"},{"line_number":190,"context_line":"        self.conj_id_rule_map \u003d dict()"},{"line_number":191,"context_line":"        self._drop_all_unmatched_flows()"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def apply_port_filter(self, port):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_8d532139","line":190,"range":{"start_line":189,"start_character":0,"end_line":190,"end_character":38},"updated":"2016-06-28 13:44:31.000000000","message":"Can we come up with an object design and separate getting of conj_id out from Firewall class?","commit_id":"d6ba09ff7fd1fb3f7fe63137050f31c3c3d4f734"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"2f4aa64986d4137aa9c830d37e03b55e063bae62","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        self.int_br \u003d self.initialize_bridge(integration_bridge)"},{"line_number":187,"context_line":"        self.sg_port_map \u003d SGPortMap()"},{"line_number":188,"context_line":"        self._deferred \u003d False"},{"line_number":189,"context_line":"        self.conj_id_map \u003d dict()"},{"line_number":190,"context_line":"        self.conj_id_rule_map \u003d dict()"},{"line_number":191,"context_line":"        self._drop_all_unmatched_flows()"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def apply_port_filter(self, port):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad45d7e_6987d361","line":190,"range":{"start_line":189,"start_character":0,"end_line":190,"end_character":38},"in_reply_to":"3aaa91ec_0fa9c27b","updated":"2016-08-09 04:34:45.000000000","message":"Done","commit_id":"d6ba09ff7fd1fb3f7fe63137050f31c3c3d4f734"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"a702d04ee1b72f0e0f7695f61c51a000331b021f","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        self.int_br \u003d self.initialize_bridge(integration_bridge)"},{"line_number":187,"context_line":"        self.sg_port_map \u003d SGPortMap()"},{"line_number":188,"context_line":"        self._deferred \u003d False"},{"line_number":189,"context_line":"        self.conj_id_map \u003d dict()"},{"line_number":190,"context_line":"        self.conj_id_rule_map \u003d dict()"},{"line_number":191,"context_line":"        self._drop_all_unmatched_flows()"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def apply_port_filter(self, port):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_0fa9c27b","line":190,"range":{"start_line":189,"start_character":0,"end_line":190,"end_character":38},"in_reply_to":"3aaa91ec_8d532139","updated":"2016-06-29 01:42:09.000000000","message":"Agreed.  Will try.","commit_id":"d6ba09ff7fd1fb3f7fe63137050f31c3c3d4f734"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"ecb0b914c3a00a95a64194b1514b016f21ddf123","unresolved":false,"context_lines":[{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        id_set \u003d set(self.id_map.values())"},{"line_number":187,"context_line":"        while True:"},{"line_number":188,"context_line":"            conj_id \u003d random.randrange(0, 0x3fff)"},{"line_number":189,"context_line":"            if conj_id not in id_set:"},{"line_number":190,"context_line":"                break"},{"line_number":191,"context_line":"        self.id_map[sg_id] \u003d conj_id"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a8bc5a2_60ae1a2f","line":188,"updated":"2016-09-27 14:31:24.000000000","message":"this will become slower as groups grow.\n\nNot sure if this is an early optimization or not, but could we create an array of free ids, and just pop from there?, and push back when released?","commit_id":"7c9d5b19b2d808b349b2ed5ab1a4b158d8e35a48"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"749972822500d93f6ea63ab9246ef8124aa02e06","unresolved":false,"context_lines":[{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        id_set \u003d set(self.id_map.values())"},{"line_number":187,"context_line":"        while True:"},{"line_number":188,"context_line":"            conj_id \u003d random.randrange(0, 0x3fff)"},{"line_number":189,"context_line":"            if conj_id not in id_set:"},{"line_number":190,"context_line":"                break"},{"line_number":191,"context_line":"        self.id_map[sg_id] \u003d conj_id"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a98d1f4_c6b6e889","line":188,"in_reply_to":"5a8bc5a2_60ae1a2f","updated":"2016-09-29 08:25:31.000000000","message":"I tried to save typing by using a randomized algo. ;)\nI think it\u0027s not very bad when there aren\u0027t many groups, but I understand your concern.\n\nWill fix in next patch.","commit_id":"7c9d5b19b2d808b349b2ed5ab1a4b158d8e35a48"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"dc9cbb05caf53796eb364c71ec368323d1029dc7","unresolved":false,"context_lines":[{"line_number":161,"context_line":"            if sec_group.vlan_tag is None:"},{"line_number":162,"context_line":"                sec_group.vlan_tag \u003d port.vlan_tag"},{"line_number":163,"context_line":"            else:"},{"line_number":164,"context_line":"                assert sec_group.vlan_tag \u003d\u003d port.vlan_tag"},{"line_number":165,"context_line":"        port.update(port_dict)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"    def remove_port(self, port):"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa6399be_c1a1edf9","line":164,"updated":"2016-10-14 02:16:11.000000000","message":"I\u0027m only 90% sure this assertion is okay.","commit_id":"9780fa72570a0b17caa67c27ef65a343f95b3ea8"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"afb9cd6b56749c5b1981e5025348a09c902224ff","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        for sec_group in port.sec_groups:"},{"line_number":158,"context_line":"            sec_group.ports.add(port)"},{"line_number":159,"context_line":"            if sec_group.vlan_tag is None:"},{"line_number":160,"context_line":"                sec_group.vlan_tag \u003d port.vlan_tag"},{"line_number":161,"context_line":"            else:"},{"line_number":162,"context_line":"                assert sec_group.vlan_tag \u003d\u003d port.vlan_tag"},{"line_number":163,"context_line":"        port.update(port_dict)"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba5da102_d56b67b7","line":160,"updated":"2016-11-02 13:30:42.000000000","message":"hmmm, you can have a security group with ports from different networks (so different local vlans)\n\nFor example, default group.\n\nPort belonging to two different networks (private1 \u0026 private2) will have different vlan ids, but will be on the same security group.\n\nI think this assumption is wrong.","commit_id":"e9556dc5a1b6166dbfa1d824ab8c3da4a6ffb807"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"355f4afa0dd56e3d97ee4cce936f1b788e0926ab","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        for sec_group in port.sec_groups:"},{"line_number":158,"context_line":"            sec_group.ports.add(port)"},{"line_number":159,"context_line":"            if sec_group.vlan_tag is None:"},{"line_number":160,"context_line":"                sec_group.vlan_tag \u003d port.vlan_tag"},{"line_number":161,"context_line":"            else:"},{"line_number":162,"context_line":"                assert sec_group.vlan_tag \u003d\u003d port.vlan_tag"},{"line_number":163,"context_line":"        port.update(port_dict)"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba5da102_fbb72bf3","line":160,"in_reply_to":"ba5da102_d56b67b7","updated":"2016-11-04 06:18:54.000000000","message":"I\u0027m using reg_net\u003dvlan_tag match for remote_group based flow rules (see create_flows_for_ip_address), and need reg_net values to be able to delete flows.\n\nI guess I can use a list of vlan_tags here, but I need to think a bit more.","commit_id":"e9556dc5a1b6166dbfa1d824ab8c3da4a6ffb807"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"e0414b7742b1d1cdabce7fcb1cee605bae2b2ad0","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        for sec_group in port.sec_groups:"},{"line_number":158,"context_line":"            sec_group.ports.add(port)"},{"line_number":159,"context_line":"            if sec_group.vlan_tag is None:"},{"line_number":160,"context_line":"                sec_group.vlan_tag \u003d port.vlan_tag"},{"line_number":161,"context_line":"            else:"},{"line_number":162,"context_line":"                assert sec_group.vlan_tag \u003d\u003d port.vlan_tag"},{"line_number":163,"context_line":"        port.update(port_dict)"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a6eadb0_80fea069","line":160,"in_reply_to":"ba5da102_fbb72bf3","updated":"2016-12-13 08:21:30.000000000","message":"Done","commit_id":"e9556dc5a1b6166dbfa1d824ab8c3da4a6ffb807"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"30823d1701bad070f62cfcfc2d526bc3527fae3c","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":217,"context_line":"        | 0 |          X          |          Y          | Z | W | 0 |"},{"line_number":218,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":219,"context_line":"        X and Y are 14 bits wide and are mapped to remote_sg_id and"},{"line_number":220,"context_line":"        sg_id, respectively."},{"line_number":221,"context_line":"        Z is 0 for ingress and 1 for egress rules."},{"line_number":222,"context_line":"        W is 0 for IPv4 and 1 for IPv6."}],"source_content_type":"text/x-python","patch_set":14,"id":"ba5da102_a6e5fddc","line":219,"updated":"2016-11-02 06:08:07.000000000","message":"Hi Jakub,\n\nif we do self.id_map[(sg_id, remote_id, direction, ethertype)] thing to avoid this bit operation, self.id_free would need to be very large, hitting performance.\n\nOTOH, we are probably not going to host 100000 sg rules on an agent so limiting conj_id range would be another solution.","commit_id":"e9556dc5a1b6166dbfa1d824ab8c3da4a6ffb807"},{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"407e5da138c324f0c7aae840198cb6b29564ff0c","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        self.remote_rules \u003d [rule for rule in rules"},{"line_number":76,"context_line":"                             if \u0027remote_group_id\u0027 in rule]"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def get_ethertype_filtered_addresses(self, ethertype):"},{"line_number":79,"context_line":"        return self.members.get(ethertype, [])"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def get_addresses_on_flows(self, ethertype):"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_1ed19cfa","line":78,"updated":"2016-11-17 13:55:20.000000000","message":"The API changed here is used in the tests: neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py ( https://review.openstack.org/gitweb?p\u003dopenstack/neutron.git;a\u003dblob;f\u003dneutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py;h\u003d809985b17a096e76d077f9e6a407fab020e61b91;hb\u003d5805e22073a48f820e281242d4f347f348c5f41d#l73 )","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"fadbc5525ca8ee3f9bb24fbdea49fb6bc5482085","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        self.remote_rules \u003d [rule for rule in rules"},{"line_number":76,"context_line":"                             if \u0027remote_group_id\u0027 in rule]"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def get_ethertype_filtered_addresses(self, ethertype):"},{"line_number":79,"context_line":"        return self.members.get(ethertype, [])"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def get_addresses_on_flows(self, ethertype):"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_f33dd58f","line":78,"in_reply_to":"7a77a97e_1ed19cfa","updated":"2016-11-18 06:38:50.000000000","message":"Oh these tests haven\u0027t run because their names don\u0027t start with \"test\".\nDone.","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"407e5da138c324f0c7aae840198cb6b29564ff0c","unresolved":false,"context_lines":[{"line_number":179,"context_line":"    def __init__(self):"},{"line_number":180,"context_line":"        self.id_map \u003d dict()"},{"line_number":181,"context_line":"        self.id_free \u003d list(range(0x4000))"},{"line_number":182,"context_line":"        self.id_in_use \u003d dict()"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def _sg_id_to_conj(self, sg_id):"},{"line_number":185,"context_line":"        conj_id \u003d self.id_map.get(sg_id)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_ed0454c1","line":182,"updated":"2016-11-17 13:55:20.000000000","message":"Maybe use collections.defaultdict for id_map and id_in_use?","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"fadbc5525ca8ee3f9bb24fbdea49fb6bc5482085","unresolved":false,"context_lines":[{"line_number":179,"context_line":"    def __init__(self):"},{"line_number":180,"context_line":"        self.id_map \u003d dict()"},{"line_number":181,"context_line":"        self.id_free \u003d list(range(0x4000))"},{"line_number":182,"context_line":"        self.id_in_use \u003d dict()"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def _sg_id_to_conj(self, sg_id):"},{"line_number":185,"context_line":"        conj_id \u003d self.id_map.get(sg_id)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_ee2327bb","line":182,"in_reply_to":"7a77a97e_ed0454c1","updated":"2016-11-18 06:38:50.000000000","message":"Done","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"407e5da138c324f0c7aae840198cb6b29564ff0c","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        if conj_id is not None:"},{"line_number":187,"context_line":"            return conj_id"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        conj_id \u003d self.id_free.pop()"},{"line_number":190,"context_line":"        self.id_map[sg_id] \u003d conj_id"},{"line_number":191,"context_line":"        return conj_id"},{"line_number":192,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_de3e44d9","line":189,"updated":"2016-11-17 13:55:20.000000000","message":"What happens when you run out of IDs? Can that happen?","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"fadbc5525ca8ee3f9bb24fbdea49fb6bc5482085","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        if conj_id is not None:"},{"line_number":187,"context_line":"            return conj_id"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        conj_id \u003d self.id_free.pop()"},{"line_number":190,"context_line":"        self.id_map[sg_id] \u003d conj_id"},{"line_number":191,"context_line":"        return conj_id"},{"line_number":192,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_73a2a59a","line":189,"in_reply_to":"7a77a97e_de3e44d9","updated":"2016-11-18 06:38:50.000000000","message":"Bad thing will happen if there are more than 16384 active security groups on a compute node.\n\nI don\u0027t think that\u0027s realistic.","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"407e5da138c324f0c7aae840198cb6b29564ff0c","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        conj_id |\u003d self._sg_id_to_conj(sg_id) \u003c\u003c 3"},{"line_number":231,"context_line":"        conj_id |\u003d self._calc_extra_bits(direction, ethertype)"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"        in_use_key \u003d (remote_sg_id, conj_id \u0026 (4 | 2))"},{"line_number":234,"context_line":"        if in_use_key not in self.id_in_use:"},{"line_number":235,"context_line":"            self.id_in_use[in_use_key] \u003d set()"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"        self.id_in_use[in_use_key].add(conj_id)"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"        return conj_id"},{"line_number":240,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_d15b95f4","line":237,"range":{"start_line":233,"start_character":0,"end_line":237,"end_character":47},"updated":"2016-11-17 13:55:20.000000000","message":"If you make id_in_use as a default dict, you could do:\nget_sibling_conj_id(remote_sg_id, direction, ethertype).add(conj_id) .\n\nWould also mean you calculate sibling id in a single location (get_sibling_conj_id) rather than here as well.","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"fadbc5525ca8ee3f9bb24fbdea49fb6bc5482085","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        conj_id |\u003d self._sg_id_to_conj(sg_id) \u003c\u003c 3"},{"line_number":231,"context_line":"        conj_id |\u003d self._calc_extra_bits(direction, ethertype)"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"        in_use_key \u003d (remote_sg_id, conj_id \u0026 (4 | 2))"},{"line_number":234,"context_line":"        if in_use_key not in self.id_in_use:"},{"line_number":235,"context_line":"            self.id_in_use[in_use_key] \u003d set()"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"        self.id_in_use[in_use_key].add(conj_id)"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"        return conj_id"},{"line_number":240,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_6ef4775a","line":237,"range":{"start_line":233,"start_character":0,"end_line":237,"end_character":47},"in_reply_to":"7a77a97e_d15b95f4","updated":"2016-11-18 06:38:50.000000000","message":"I suppose id_in_use to be sparse, so I don\u0027t like filling up it with empty sets.","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"407e5da138c324f0c7aae840198cb6b29564ff0c","unresolved":false,"context_lines":[{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def _cleanup_stale_sg(self):"},{"line_number":447,"context_line":"        for sg_id in self.sg_to_delete:"},{"line_number":448,"context_line":"            sec_group \u003d self.sg_port_map.get_or_create_sg(sg_id)"},{"line_number":449,"context_line":"            if sec_group.members and sec_group.ports:"},{"line_number":450,"context_line":"                # sec_group is still in use"},{"line_number":451,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_b439a7f4","line":448,"range":{"start_line":448,"start_character":45,"end_line":448,"end_character":54},"updated":"2016-11-17 13:55:20.000000000","message":"If the stale sg doesn\u0027t exist, wouldn\u0027t it be better to be notified and skip it here? Is that scenario even possible?","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"fadbc5525ca8ee3f9bb24fbdea49fb6bc5482085","unresolved":false,"context_lines":[{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def _cleanup_stale_sg(self):"},{"line_number":447,"context_line":"        for sg_id in self.sg_to_delete:"},{"line_number":448,"context_line":"            sec_group \u003d self.sg_port_map.get_or_create_sg(sg_id)"},{"line_number":449,"context_line":"            if sec_group.members and sec_group.ports:"},{"line_number":450,"context_line":"                # sec_group is still in use"},{"line_number":451,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_b170b2a5","line":448,"range":{"start_line":448,"start_character":45,"end_line":448,"end_character":54},"in_reply_to":"7a77a97e_b439a7f4","updated":"2016-11-18 06:38:50.000000000","message":"It should always be present here, but tweaked code in hope of removing confusion.","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"407e5da138c324f0c7aae840198cb6b29564ff0c","unresolved":false,"context_lines":[{"line_number":830,"context_line":"        self._initialize_tracked_egress(port)"},{"line_number":831,"context_line":"        LOG.debug(\u0027Creating flow rules for port %s that is port %d in OVS\u0027,"},{"line_number":832,"context_line":"                  port.id, port.ofport)"},{"line_number":833,"context_line":"        rules_gererator \u003d self._create_rules_generator_for_port(port)"},{"line_number":834,"context_line":"        for rule in rules_gererator:"},{"line_number":835,"context_line":"            flows \u003d rules.create_flows_from_rule_and_port(rule, port)"},{"line_number":836,"context_line":"            LOG.debug(\"RULGEN: Rules generated for flow %s are %s\","}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_6febb813","line":833,"range":{"start_line":833,"start_character":16,"end_line":833,"end_character":17},"updated":"2016-11-17 13:55:20.000000000","message":"Why make this change?","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"fadbc5525ca8ee3f9bb24fbdea49fb6bc5482085","unresolved":false,"context_lines":[{"line_number":830,"context_line":"        self._initialize_tracked_egress(port)"},{"line_number":831,"context_line":"        LOG.debug(\u0027Creating flow rules for port %s that is port %d in OVS\u0027,"},{"line_number":832,"context_line":"                  port.id, port.ofport)"},{"line_number":833,"context_line":"        rules_gererator \u003d self._create_rules_generator_for_port(port)"},{"line_number":834,"context_line":"        for rule in rules_gererator:"},{"line_number":835,"context_line":"            flows \u003d rules.create_flows_from_rule_and_port(rule, port)"},{"line_number":836,"context_line":"            LOG.debug(\"RULGEN: Rules generated for flow %s are %s\","}],"source_content_type":"text/x-python","patch_set":15,"id":"7a77a97e_64c9f958","line":833,"range":{"start_line":833,"start_character":16,"end_line":833,"end_character":17},"in_reply_to":"7a77a97e_6febb813","updated":"2016-11-18 06:38:50.000000000","message":"Blame my fat fingers.\n\nThe latter change is to make clear this method is for internal use.","commit_id":"5805e22073a48f820e281242d4f347f348c5f41d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c8fd83a24eea0e6a8488b3ba76292c567df26de4","unresolved":false,"context_lines":[{"line_number":182,"context_line":"class ConjIdMap(object):"},{"line_number":183,"context_line":"    def __init__(self):"},{"line_number":184,"context_line":"        self.id_map \u003d collections.defaultdict(self._conj_id_factory)"},{"line_number":185,"context_line":"        self.id_free \u003d list(range(0x4000))"},{"line_number":186,"context_line":"        self.id_in_use \u003d collections.defaultdict(set)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _conj_id_factory(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_b4dd9572","line":185,"range":{"start_line":185,"start_character":8,"end_line":185,"end_character":42},"updated":"2016-12-13 10:57:07.000000000","message":"small nit: for sure you expend time thinking this class and the inside algorithms, but it\u0027s a bit expensive to initialize 16K integers for free.","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"ecdf1f5f69b88bbaab1c922256307038da7f7866","unresolved":false,"context_lines":[{"line_number":182,"context_line":"class ConjIdMap(object):"},{"line_number":183,"context_line":"    def __init__(self):"},{"line_number":184,"context_line":"        self.id_map \u003d collections.defaultdict(self._conj_id_factory)"},{"line_number":185,"context_line":"        self.id_free \u003d list(range(0x4000))"},{"line_number":186,"context_line":"        self.id_in_use \u003d collections.defaultdict(set)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _conj_id_factory(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_ddef3baf","line":185,"range":{"start_line":185,"start_character":8,"end_line":185,"end_character":42},"in_reply_to":"1a6eadb0_b4dd9572","updated":"2016-12-21 07:31:47.000000000","message":"It happens only once.\nlist(range(0x4000 * 1000)) takes \u003c1 sec on a 3GHz machine, so the above should take less than 1 ms.","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c8fd83a24eea0e6a8488b3ba76292c567df26de4","unresolved":false,"context_lines":[{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _conj_id_factory(self):"},{"line_number":189,"context_line":"        conj_id \u003d self.id_free.pop()"},{"line_number":190,"context_line":"        return conj_id"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    def _calc_extra_bits(self, direction, ethertype):"},{"line_number":193,"context_line":"        bits \u003d 0"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_8349458e","line":190,"range":{"start_line":190,"start_character":8,"end_line":190,"end_character":22},"updated":"2016-12-13 10:57:07.000000000","message":"You can directly do:\n  return self.id_free.pop()","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"ecdf1f5f69b88bbaab1c922256307038da7f7866","unresolved":false,"context_lines":[{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _conj_id_factory(self):"},{"line_number":189,"context_line":"        conj_id \u003d self.id_free.pop()"},{"line_number":190,"context_line":"        return conj_id"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    def _calc_extra_bits(self, direction, ethertype):"},{"line_number":193,"context_line":"        bits \u003d 0"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_fd4317ad","line":190,"range":{"start_line":190,"start_character":8,"end_line":190,"end_character":22},"in_reply_to":"1a6eadb0_8349458e","updated":"2016-12-21 07:31:47.000000000","message":"Done","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c8fd83a24eea0e6a8488b3ba76292c567df26de4","unresolved":false,"context_lines":[{"line_number":205,"context_line":"        direction \u003d (firewall.EGRESS_DIRECTION if extra_bits \u0026 4 else"},{"line_number":206,"context_line":"                     firewall.INGRESS_DIRECTION)"},{"line_number":207,"context_line":"        ethertype \u003d lib_const.IPv6 if extra_bits \u0026 2 else lib_const.IPv4"},{"line_number":208,"context_line":"        return (direction, ethertype)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    def get_conj_id(self, sg_id, remote_sg_id, direction, ethertype):"},{"line_number":211,"context_line":"        \"\"\"Return a conjunction ID, allocating one if not present."}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_2026a7ba","line":208,"range":{"start_line":208,"start_character":15,"end_line":208,"end_character":37},"updated":"2016-12-13 10:57:07.000000000","message":"You should remove the parentheses","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"ecdf1f5f69b88bbaab1c922256307038da7f7866","unresolved":false,"context_lines":[{"line_number":205,"context_line":"        direction \u003d (firewall.EGRESS_DIRECTION if extra_bits \u0026 4 else"},{"line_number":206,"context_line":"                     firewall.INGRESS_DIRECTION)"},{"line_number":207,"context_line":"        ethertype \u003d lib_const.IPv6 if extra_bits \u0026 2 else lib_const.IPv4"},{"line_number":208,"context_line":"        return (direction, ethertype)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    def get_conj_id(self, sg_id, remote_sg_id, direction, ethertype):"},{"line_number":211,"context_line":"        \"\"\"Return a conjunction ID, allocating one if not present."}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_1d1373b9","line":208,"range":{"start_line":208,"start_character":15,"end_line":208,"end_character":37},"in_reply_to":"1a6eadb0_2026a7ba","updated":"2016-12-21 07:31:47.000000000","message":"Done","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c8fd83a24eea0e6a8488b3ba76292c567df26de4","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":215,"context_line":"        | 0 |          X          |          Y          | Z | W | 0 |"},{"line_number":216,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":217,"context_line":"        X and Y are 14 bits wide and are mapped to remote_sg_id and"},{"line_number":218,"context_line":"        sg_id, respectively."},{"line_number":219,"context_line":"        Z is 0 for ingress and 1 for egress rules."},{"line_number":220,"context_line":"        W is 0 for IPv4 and 1 for IPv6."},{"line_number":221,"context_line":"        The LSB is used to handle 2 flows that differs in ct_state."}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_f97e26ad","line":218,"range":{"start_line":217,"start_character":7,"end_line":218,"end_character":28},"updated":"2016-12-13 10:57:07.000000000","message":"If you are using the ID, why are you limiting this to 14 bits?","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"ecdf1f5f69b88bbaab1c922256307038da7f7866","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":215,"context_line":"        | 0 |          X          |          Y          | Z | W | 0 |"},{"line_number":216,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":217,"context_line":"        X and Y are 14 bits wide and are mapped to remote_sg_id and"},{"line_number":218,"context_line":"        sg_id, respectively."},{"line_number":219,"context_line":"        Z is 0 for ingress and 1 for egress rules."},{"line_number":220,"context_line":"        W is 0 for IPv4 and 1 for IPv6."},{"line_number":221,"context_line":"        The LSB is used to handle 2 flows that differs in ct_state."}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_3d666f51","line":218,"range":{"start_line":217,"start_character":7,"end_line":218,"end_character":28},"in_reply_to":"1a6eadb0_f97e26ad","updated":"2016-12-21 07:31:47.000000000","message":"Because conjunction ids are 32 bits wide.\nI think 14 bits is more than enough. How about you?","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c8fd83a24eea0e6a8488b3ba76292c567df26de4","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        # The purpose of self.id_in_use is keep those sizes and flow"},{"line_number":244,"context_line":"        # action sizes small."},{"line_number":245,"context_line":"        in_use_key \u003d (remote_sg_id,"},{"line_number":246,"context_line":"                      self._calc_extra_bits(direction, ethertype))"},{"line_number":247,"context_line":"        return self.id_in_use.get(in_use_key, set())"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def delete_sg(self, sg_id):"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_e3fcb940","line":246,"range":{"start_line":246,"start_character":22,"end_line":246,"end_character":65},"updated":"2016-12-13 10:57:07.000000000","message":"Please, correct me if I\u0027m wrong (the code is a bit complex).\n\nIf you are creating the \"in_use_key\" set second parameter only with the extra bits and in L232 you are using \"conj_id \u0026 (4 | 2)\", you\u0027ll never find any used key...","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"ecdf1f5f69b88bbaab1c922256307038da7f7866","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        # The purpose of self.id_in_use is keep those sizes and flow"},{"line_number":244,"context_line":"        # action sizes small."},{"line_number":245,"context_line":"        in_use_key \u003d (remote_sg_id,"},{"line_number":246,"context_line":"                      self._calc_extra_bits(direction, ethertype))"},{"line_number":247,"context_line":"        return self.id_in_use.get(in_use_key, set())"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def delete_sg(self, sg_id):"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_bd00bf35","line":246,"range":{"start_line":246,"start_character":22,"end_line":246,"end_character":65},"in_reply_to":"1a6eadb0_e3fcb940","updated":"2016-12-21 07:31:47.000000000","message":"I removed this method in the current version.\n\nconj_id \u0026 (4 | 2) is where _calc_extra_bits bits go. See L230.","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c8fd83a24eea0e6a8488b3ba76292c567df26de4","unresolved":false,"context_lines":[{"line_number":254,"context_line":"        if sg_id not in self.id_map:"},{"line_number":255,"context_line":"            return []"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        numeric_sg_id \u003d self.id_map[sg_id]"},{"line_number":258,"context_line":"        del self.id_map[sg_id]"},{"line_number":259,"context_line":"        for extra_bits in [0, 2, 4, 6]:"},{"line_number":260,"context_line":"            in_use_key \u003d (sg_id, extra_bits)"},{"line_number":261,"context_line":"            self.id_in_use.pop(in_use_key, None)"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_39a09ef4","line":258,"range":{"start_line":257,"start_character":0,"end_line":258,"end_character":30},"updated":"2016-12-13 10:57:07.000000000","message":"You can pop the value using the key:\n    numeric_sg_id \u003d self.id_map.pop(sg_id)","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"ecdf1f5f69b88bbaab1c922256307038da7f7866","unresolved":false,"context_lines":[{"line_number":254,"context_line":"        if sg_id not in self.id_map:"},{"line_number":255,"context_line":"            return []"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        numeric_sg_id \u003d self.id_map[sg_id]"},{"line_number":258,"context_line":"        del self.id_map[sg_id]"},{"line_number":259,"context_line":"        for extra_bits in [0, 2, 4, 6]:"},{"line_number":260,"context_line":"            in_use_key \u003d (sg_id, extra_bits)"},{"line_number":261,"context_line":"            self.id_in_use.pop(in_use_key, None)"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_7d3d47dc","line":258,"range":{"start_line":257,"start_character":0,"end_line":258,"end_character":30},"in_reply_to":"1a6eadb0_39a09ef4","updated":"2016-12-21 07:31:47.000000000","message":"Done","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c8fd83a24eea0e6a8488b3ba76292c567df26de4","unresolved":false,"context_lines":[{"line_number":839,"context_line":"        updated_remote_rules \u003d set()"},{"line_number":840,"context_line":"        remote_rules_generator \u003d ("},{"line_number":841,"context_line":"            self._create_remote_rules_generator_for_port(port))"},{"line_number":842,"context_line":"        for sec_group_id, rule in remote_rules_generator:"},{"line_number":843,"context_line":"            direction \u003d rule[\u0027direction\u0027]"},{"line_number":844,"context_line":"            ethertype \u003d rule[\u0027ethertype\u0027]"},{"line_number":845,"context_line":"            conj_id \u003d self.conj_id_map.get_conj_id("}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_4cd03f54","line":842,"range":{"start_line":842,"start_character":34,"end_line":842,"end_character":56},"updated":"2016-12-13 10:57:07.000000000","message":"You can write here 2self._create_remote_rules_generator_for_port(port)\" directly.\n\nYou are not using variable \"remote_rules_generator\" anymore","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"ecdf1f5f69b88bbaab1c922256307038da7f7866","unresolved":false,"context_lines":[{"line_number":839,"context_line":"        updated_remote_rules \u003d set()"},{"line_number":840,"context_line":"        remote_rules_generator \u003d ("},{"line_number":841,"context_line":"            self._create_remote_rules_generator_for_port(port))"},{"line_number":842,"context_line":"        for sec_group_id, rule in remote_rules_generator:"},{"line_number":843,"context_line":"            direction \u003d rule[\u0027direction\u0027]"},{"line_number":844,"context_line":"            ethertype \u003d rule[\u0027ethertype\u0027]"},{"line_number":845,"context_line":"            conj_id \u003d self.conj_id_map.get_conj_id("}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_3853dd18","line":842,"range":{"start_line":842,"start_character":34,"end_line":842,"end_character":56},"in_reply_to":"1a6eadb0_4cd03f54","updated":"2016-12-21 07:31:47.000000000","message":"Done","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"304bd132b5da3ec0eb345bea2e1c7cba509c0127","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import collections"},{"line_number":17,"context_line":"import netaddr"},{"line_number":18,"context_line":"from neutron_lib import constants as lib_const"},{"line_number":19,"context_line":"from neutron_lib import exceptions"}],"source_content_type":"text/x-python","patch_set":19,"id":"da4df55a_fe78dafa","line":16,"updated":"2016-12-22 10:06:22.000000000","message":"this is not third party so should be in its own section","commit_id":"765f675c16854458b7ba60fcc1e264f72aae7701"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"5a243435bd07d166149b8db35340a4d4aed7fc35","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import collections"},{"line_number":17,"context_line":"import netaddr"},{"line_number":18,"context_line":"from neutron_lib import constants as lib_const"},{"line_number":19,"context_line":"from neutron_lib import exceptions"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_bbc8a9a9","line":16,"in_reply_to":"da4df55a_fe78dafa","updated":"2017-01-05 05:34:28.000000000","message":"Done","commit_id":"765f675c16854458b7ba60fcc1e264f72aae7701"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"081ca4b854a785f3c6d3fe65f48e93c3e61ec72e","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":"        A conjunction ID is 32 bits long and in the following format."},{"line_number":216,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":217,"context_line":"        | 0 |          X          |          Y          | Z | W | 0 |"},{"line_number":218,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":219,"context_line":"        X and Y are 14 bits wide and are mapped to remote_sg_id and"},{"line_number":220,"context_line":"        sg_id, respectively."}],"source_content_type":"text/x-python","patch_set":21,"id":"ba5201f7_b7022eb2","line":217,"updated":"2017-01-06 23:27:28.000000000","message":"+1. At a minimum I would want to see comments explaining where each of the magic numbers is coming from.\n\ne.g. L193 is just called \u0027_calc_extra_bits\u0027 and it does something if it\u0027s ingress and something if it\u0027s IPv6, with no explanation why or what the bits being set represent.","commit_id":"fcecc59de2b7fae0daa02d19ec3ee4640beaa1f2"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"1c207a1205c3c5fa0c90c12cfd95d467b4780fe1","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":"        A conjunction ID is 32 bits long and in the following format."},{"line_number":216,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":217,"context_line":"        | 0 |          X          |          Y          | Z | W | 0 |"},{"line_number":218,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":219,"context_line":"        X and Y are 14 bits wide and are mapped to remote_sg_id and"},{"line_number":220,"context_line":"        sg_id, respectively."}],"source_content_type":"text/x-python","patch_set":21,"id":"ba5201f7_f1c07e52","line":217,"range":{"start_line":217,"start_character":0,"end_line":217,"end_character":69},"updated":"2017-01-05 15:30:13.000000000","message":"I still don\u0027t understand why do we need to encode remote_sg_id, sg_id, direction and ethertype to conj_id. Where do you extract this information from conj_id? Can\u0027t be conjunction an object with this information?","commit_id":"fcecc59de2b7fae0daa02d19ec3ee4640beaa1f2"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e668f9481609537627b3859e1295b817d8dbcbfc","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":"        A conjunction ID is 32 bits long and in the following format."},{"line_number":216,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":217,"context_line":"        | 0 |          X          |          Y          | Z | W | 0 |"},{"line_number":218,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":219,"context_line":"        X and Y are 14 bits wide and are mapped to remote_sg_id and"},{"line_number":220,"context_line":"        sg_id, respectively."}],"source_content_type":"text/x-python","patch_set":21,"id":"ba5201f7_0d714976","line":217,"range":{"start_line":217,"start_character":0,"end_line":217,"end_character":69},"in_reply_to":"ba5201f7_18e7cd40","updated":"2017-01-06 10:16:49.000000000","message":"It\u0027s better to implement such things with structures/objects than shifting bits using magical constants (L207, L209, L229, L230, L233). Readability and maintainability is also important even if you need to sacrifice couple of more bytes in memory. If memory consumption becomes too big, we can optimise later, otherwise this is premature optimisation.","commit_id":"fcecc59de2b7fae0daa02d19ec3ee4640beaa1f2"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"7ff8b3f63fe379b46b06945902a873d4c9b0799f","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":"        A conjunction ID is 32 bits long and in the following format."},{"line_number":216,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":217,"context_line":"        | 0 |          X          |          Y          | Z | W | 0 |"},{"line_number":218,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":219,"context_line":"        X and Y are 14 bits wide and are mapped to remote_sg_id and"},{"line_number":220,"context_line":"        sg_id, respectively."}],"source_content_type":"text/x-python","patch_set":21,"id":"7a3c09a3_ebd49b48","line":217,"in_reply_to":"ba5201f7_47582257","updated":"2017-01-13 09:26:10.000000000","message":"I\u0027m an old fart used to shifting bits, that\u0027s why this didn\u0027t hurt my eyes, but brought comfort ;)\n\nI\u0027m happy with a tuple based key map too.","commit_id":"fcecc59de2b7fae0daa02d19ec3ee4640beaa1f2"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"1d7b8f90611aeb18da97f5ddb5a1e244526b1b00","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":"        A conjunction ID is 32 bits long and in the following format."},{"line_number":216,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":217,"context_line":"        | 0 |          X          |          Y          | Z | W | 0 |"},{"line_number":218,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":219,"context_line":"        X and Y are 14 bits wide and are mapped to remote_sg_id and"},{"line_number":220,"context_line":"        sg_id, respectively."}],"source_content_type":"text/x-python","patch_set":21,"id":"ba5201f7_47582257","line":217,"in_reply_to":"ba5201f7_b7022eb2","updated":"2017-01-10 09:04:03.000000000","message":"The code iterates over (sg_id, remote_sg_id) in L.254 so it\u0027s premature optimization anyway.\n\nLearning how this code is unpopular, I\u0027ve replaced with a simpler implementation.","commit_id":"fcecc59de2b7fae0daa02d19ec3ee4640beaa1f2"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"bb3ac8d53db444b95f268a225012a2bed7baeb06","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":"        A conjunction ID is 32 bits long and in the following format."},{"line_number":216,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":217,"context_line":"        | 0 |          X          |          Y          | Z | W | 0 |"},{"line_number":218,"context_line":"        +---+---------------------+---------------------+---+---+---+"},{"line_number":219,"context_line":"        X and Y are 14 bits wide and are mapped to remote_sg_id and"},{"line_number":220,"context_line":"        sg_id, respectively."}],"source_content_type":"text/x-python","patch_set":21,"id":"ba5201f7_18e7cd40","line":217,"range":{"start_line":217,"start_character":0,"end_line":217,"end_character":69},"in_reply_to":"ba5201f7_f1c07e52","updated":"2017-01-06 04:43:56.000000000","message":"Other than providing the siblings functionality (which is no longer needed), my original intention was to keep the id_map small, but that\u0027s probably not essential as id_in_use holds all (remote_sg_id, sg_id, d, e) combinations anyway.\nStill I think it\u0027s natural to design conj_id with this bit fields separation so that each bit field corresponds to each component of (remoto_sg_id, sg_id, d, e) tuple.\n\nI think this implementation isn\u0027t so complicated.\nIf this bit operation really hurts, we may be able to provide the same functionality (including delete_sg) using (remote_sg_id, sg_id, d, e) as a key for id_map instead and without this bit operation.","commit_id":"fcecc59de2b7fae0daa02d19ec3ee4640beaa1f2"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    def _conj_id_factory(self):"},{"line_number":179,"context_line":"        if self.id_free:"},{"line_number":180,"context_line":"            return self.id_free.popleft()"},{"line_number":181,"context_line":"        self.max_id +\u003d 2"},{"line_number":182,"context_line":"        return self.max_id"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_cee63f43","line":180,"updated":"2017-01-18 13:25:18.000000000","message":"ok, so this is a mechanism to reuse ids when freed before.","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    def _conj_id_factory(self):"},{"line_number":179,"context_line":"        if self.id_free:"},{"line_number":180,"context_line":"            return self.id_free.popleft()"},{"line_number":181,"context_line":"        self.max_id +\u003d 2"},{"line_number":182,"context_line":"        return self.max_id"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_44480b88","line":180,"in_reply_to":"7a3c09a3_cee63f43","updated":"2017-01-24 03:41:51.000000000","message":"yes","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":178,"context_line":"    def _conj_id_factory(self):"},{"line_number":179,"context_line":"        if self.id_free:"},{"line_number":180,"context_line":"            return self.id_free.popleft()"},{"line_number":181,"context_line":"        self.max_id +\u003d 2"},{"line_number":182,"context_line":"        return self.max_id"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def get_conj_id(self, sg_id, remote_sg_id, direction, ethertype):"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_0ed3e760","line":181,"updated":"2017-01-18 13:25:18.000000000","message":"why +\u003d2 ?","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":178,"context_line":"    def _conj_id_factory(self):"},{"line_number":179,"context_line":"        if self.id_free:"},{"line_number":180,"context_line":"            return self.id_free.popleft()"},{"line_number":181,"context_line":"        self.max_id +\u003d 2"},{"line_number":182,"context_line":"        return self.max_id"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def get_conj_id(self, sg_id, remote_sg_id, direction, ethertype):"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_044e0396","line":181,"in_reply_to":"7a3c09a3_0ed3e760","updated":"2017-01-24 03:41:51.000000000","message":"Because get_conj_id must return an even number.","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":215,"context_line":"        nw_src\u003d10.2.3.4,reg_net\u003d0xf00 actions\u003dconjunction(123,1/2)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    These flows are managed per network and are usually per remote_group_id,"},{"line_number":218,"context_line":"    but flows from different remote_group need to be merged on shared networks,"},{"line_number":219,"context_line":"    where the complexity arises and this manager is needed."},{"line_number":220,"context_line":"    \"\"\""},{"line_number":221,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_4e432f6c","line":218,"updated":"2017-01-18 13:25:18.000000000","message":"nice docstring :)","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        \"\"\"Install action\u003dconjunction(conj_id, 1/2) flows,"},{"line_number":231,"context_line":"        which depend on IP addresses of remote_group_id."},{"line_number":232,"context_line":"        \"\"\""},{"line_number":233,"context_line":"        for k, v in self.conj_ids[vlan_tag].items():"},{"line_number":234,"context_line":"            # The followings are abbrevs for direction and ethertype."},{"line_number":235,"context_line":"            di, et \u003d k"},{"line_number":236,"context_line":"            if (di, et) not in self.flow_state[vlan_tag]:"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_e95cd51a","line":233,"updated":"2017-01-18 13:25:18.000000000","message":"nit: I\u0027d avoid abbrevs k/v\nk could be direction_ethertype\nv could be sg_conj_id_map ?","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        \"\"\"Install action\u003dconjunction(conj_id, 1/2) flows,"},{"line_number":231,"context_line":"        which depend on IP addresses of remote_group_id."},{"line_number":232,"context_line":"        \"\"\""},{"line_number":233,"context_line":"        for k, v in self.conj_ids[vlan_tag].items():"},{"line_number":234,"context_line":"            # The followings are abbrevs for direction and ethertype."},{"line_number":235,"context_line":"            di, et \u003d k"},{"line_number":236,"context_line":"            if (di, et) not in self.flow_state[vlan_tag]:"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_9d2166b7","line":233,"in_reply_to":"7a3c09a3_e95cd51a","updated":"2017-01-24 03:41:51.000000000","message":"Done.\n\nI opted for less line-wraps, but that was because I looked at this patch for too long. ;)","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        \"\"\""},{"line_number":233,"context_line":"        for k, v in self.conj_ids[vlan_tag].items():"},{"line_number":234,"context_line":"            # The followings are abbrevs for direction and ethertype."},{"line_number":235,"context_line":"            di, et \u003d k"},{"line_number":236,"context_line":"            if (di, et) not in self.flow_state[vlan_tag]:"},{"line_number":237,"context_line":"                self.flow_state[vlan_tag][(di, et)] \u003d {}"},{"line_number":238,"context_line":"            # Build a map of addr -\u003e list of conj_ids"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_29a17df8","line":235,"updated":"2017-01-18 13:25:18.000000000","message":"nit: same for di and et.","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        \"\"\""},{"line_number":233,"context_line":"        for k, v in self.conj_ids[vlan_tag].items():"},{"line_number":234,"context_line":"            # The followings are abbrevs for direction and ethertype."},{"line_number":235,"context_line":"            di, et \u003d k"},{"line_number":236,"context_line":"            if (di, et) not in self.flow_state[vlan_tag]:"},{"line_number":237,"context_line":"                self.flow_state[vlan_tag][(di, et)] \u003d {}"},{"line_number":238,"context_line":"            # Build a map of addr -\u003e list of conj_ids"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_bd30e202","line":235,"in_reply_to":"7a3c09a3_29a17df8","updated":"2017-01-24 03:41:51.000000000","message":"Done","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        for k, v in self.conj_ids[vlan_tag].items():"},{"line_number":234,"context_line":"            # The followings are abbrevs for direction and ethertype."},{"line_number":235,"context_line":"            di, et \u003d k"},{"line_number":236,"context_line":"            if (di, et) not in self.flow_state[vlan_tag]:"},{"line_number":237,"context_line":"                self.flow_state[vlan_tag][(di, et)] \u003d {}"},{"line_number":238,"context_line":"            # Build a map of addr -\u003e list of conj_ids"},{"line_number":239,"context_line":"            # TODO(toshii): optimize when remote_groups have"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_a9f72dd1","line":236,"updated":"2017-01-18 13:25:18.000000000","message":"nits: you could save those two lines if you define flow_state as:\n\ncollections.defaultdict(functools.partial(collections.defaultdict, dict))\n\n\n      \u003e\u003e\u003e x \u003d collections.defaultdict(functools.partial(collections.defaultdict, dict))\n      \u003e\u003e\u003e x[4095][(\u0027egress\u0027, \u0027IPv4\u0027)]\n      {}","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        for k, v in self.conj_ids[vlan_tag].items():"},{"line_number":234,"context_line":"            # The followings are abbrevs for direction and ethertype."},{"line_number":235,"context_line":"            di, et \u003d k"},{"line_number":236,"context_line":"            if (di, et) not in self.flow_state[vlan_tag]:"},{"line_number":237,"context_line":"                self.flow_state[vlan_tag][(di, et)] \u003d {}"},{"line_number":238,"context_line":"            # Build a map of addr -\u003e list of conj_ids"},{"line_number":239,"context_line":"            # TODO(toshii): optimize when remote_groups have"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_fd18aa86","line":236,"in_reply_to":"7a3c09a3_a9f72dd1","updated":"2017-01-24 03:41:51.000000000","message":"Done","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":238,"context_line":"            # Build a map of addr -\u003e list of conj_ids"},{"line_number":239,"context_line":"            # TODO(toshii): optimize when remote_groups have"},{"line_number":240,"context_line":"            # no address overlaps."},{"line_number":241,"context_line":"            conj_id_map \u003d collections.defaultdict(list)"},{"line_number":242,"context_line":"            for remote_id, conj_id_set in v.items():"},{"line_number":243,"context_line":"                remote_group \u003d self.driver.sg_port_map.get_sg(remote_id)"},{"line_number":244,"context_line":"                if not remote_group:"},{"line_number":245,"context_line":"                    LOG.debug(\u0027No member for SG %s\u0027, remote_id)"},{"line_number":246,"context_line":"                    continue"},{"line_number":247,"context_line":"                for addr in remote_group.get_ethertype_filtered_addresses(et):"},{"line_number":248,"context_line":"                    conj_id_map[addr].extend(conj_id_set)"},{"line_number":249,"context_line":"            # and do compare-delete-update"},{"line_number":250,"context_line":"            current_ips \u003d set(self.flow_state[vlan_tag][(di, et)].keys())"},{"line_number":251,"context_line":"            self.driver.delete_flows_for_ip_addresses("}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_a9872db3","line":248,"range":{"start_line":241,"start_character":11,"end_line":248,"end_character":57},"updated":"2017-01-18 13:25:18.000000000","message":"nit: this could be refactored out to a separate private method\n\nconj_id_map \u003d self._build_addr_conj_id_map(v /or better/ sg_conj_id_map)\n\nThe 3-line comment (L239/L240 could be the docstring)","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":238,"context_line":"            # Build a map of addr -\u003e list of conj_ids"},{"line_number":239,"context_line":"            # TODO(toshii): optimize when remote_groups have"},{"line_number":240,"context_line":"            # no address overlaps."},{"line_number":241,"context_line":"            conj_id_map \u003d collections.defaultdict(list)"},{"line_number":242,"context_line":"            for remote_id, conj_id_set in v.items():"},{"line_number":243,"context_line":"                remote_group \u003d self.driver.sg_port_map.get_sg(remote_id)"},{"line_number":244,"context_line":"                if not remote_group:"},{"line_number":245,"context_line":"                    LOG.debug(\u0027No member for SG %s\u0027, remote_id)"},{"line_number":246,"context_line":"                    continue"},{"line_number":247,"context_line":"                for addr in remote_group.get_ethertype_filtered_addresses(et):"},{"line_number":248,"context_line":"                    conj_id_map[addr].extend(conj_id_set)"},{"line_number":249,"context_line":"            # and do compare-delete-update"},{"line_number":250,"context_line":"            current_ips \u003d set(self.flow_state[vlan_tag][(di, et)].keys())"},{"line_number":251,"context_line":"            self.driver.delete_flows_for_ip_addresses("}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_5d161e56","line":248,"range":{"start_line":241,"start_character":11,"end_line":248,"end_character":57},"in_reply_to":"7a3c09a3_a9872db3","updated":"2017-01-24 03:41:51.000000000","message":"Done\n\nThe mentioned optimization might be avoidance of building conj_id_map when only one SG is involved.","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                    continue"},{"line_number":247,"context_line":"                for addr in remote_group.get_ethertype_filtered_addresses(et):"},{"line_number":248,"context_line":"                    conj_id_map[addr].extend(conj_id_set)"},{"line_number":249,"context_line":"            # and do compare-delete-update"},{"line_number":250,"context_line":"            current_ips \u003d set(self.flow_state[vlan_tag][(di, et)].keys())"},{"line_number":251,"context_line":"            self.driver.delete_flows_for_ip_addresses("},{"line_number":252,"context_line":"                current_ips - set(conj_id_map.keys()), di, et, vlan_tag)"},{"line_number":253,"context_line":"            for addr, conj_ids in conj_id_map.items():"},{"line_number":254,"context_line":"                conj_ids.sort()"},{"line_number":255,"context_line":"                if self.flow_state[vlan_tag][(di, et)].get(addr) \u003d\u003d conj_ids:"},{"line_number":256,"context_line":"                    continue"},{"line_number":257,"context_line":"                for flow in rules.create_flows_for_ip_address("},{"line_number":258,"context_line":"                        addr, di, et, vlan_tag, conj_ids):"},{"line_number":259,"context_line":"                    self.driver._add_flow(**flow)"},{"line_number":260,"context_line":"            self.flow_state[vlan_tag][(di, et)] \u003d conj_id_map"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"    def add(self, vlan_tag, remote_sg_id, direction, ethertype, conj_id):"},{"line_number":263,"context_line":"        \"\"\"Notify the manager that"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_34a1d4f8","line":260,"range":{"start_line":249,"start_character":8,"end_line":260,"end_character":61},"updated":"2017-01-18 13:25:18.000000000","message":"nit: And this could finally be another sub-method.\n\nAll the suggestions are for readability of the function, I believe your comments do the work anyway.","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                    continue"},{"line_number":247,"context_line":"                for addr in remote_group.get_ethertype_filtered_addresses(et):"},{"line_number":248,"context_line":"                    conj_id_map[addr].extend(conj_id_set)"},{"line_number":249,"context_line":"            # and do compare-delete-update"},{"line_number":250,"context_line":"            current_ips \u003d set(self.flow_state[vlan_tag][(di, et)].keys())"},{"line_number":251,"context_line":"            self.driver.delete_flows_for_ip_addresses("},{"line_number":252,"context_line":"                current_ips - set(conj_id_map.keys()), di, et, vlan_tag)"},{"line_number":253,"context_line":"            for addr, conj_ids in conj_id_map.items():"},{"line_number":254,"context_line":"                conj_ids.sort()"},{"line_number":255,"context_line":"                if self.flow_state[vlan_tag][(di, et)].get(addr) \u003d\u003d conj_ids:"},{"line_number":256,"context_line":"                    continue"},{"line_number":257,"context_line":"                for flow in rules.create_flows_for_ip_address("},{"line_number":258,"context_line":"                        addr, di, et, vlan_tag, conj_ids):"},{"line_number":259,"context_line":"                    self.driver._add_flow(**flow)"},{"line_number":260,"context_line":"            self.flow_state[vlan_tag][(di, et)] \u003d conj_id_map"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"    def add(self, vlan_tag, remote_sg_id, direction, ethertype, conj_id):"},{"line_number":263,"context_line":"        \"\"\"Notify the manager that"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_13e1e7d0","line":260,"range":{"start_line":249,"start_character":8,"end_line":260,"end_character":61},"in_reply_to":"7a3c09a3_34a1d4f8","updated":"2017-01-24 03:41:51.000000000","message":"Done","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        for remote_sg_id, conj_id in id_list:"},{"line_number":283,"context_line":"            unused_dict[remote_sg_id].add(conj_id)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        for vlan_tag, v in self.conj_ids.items():"},{"line_number":286,"context_line":"            update \u003d False"},{"line_number":287,"context_line":"            for di_et, dict_set in v.items():"},{"line_number":288,"context_line":"                for remote_sg_id, unused in unused_dict.items():"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_d4e6d042","line":285,"range":{"start_line":285,"start_character":22,"end_line":285,"end_character":23},"updated":"2017-01-18 13:25:18.000000000","message":"nit: please be more explicit about v name.","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        for remote_sg_id, conj_id in id_list:"},{"line_number":283,"context_line":"            unused_dict[remote_sg_id].add(conj_id)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        for vlan_tag, v in self.conj_ids.items():"},{"line_number":286,"context_line":"            update \u003d False"},{"line_number":287,"context_line":"            for di_et, dict_set in v.items():"},{"line_number":288,"context_line":"                for remote_sg_id, unused in unused_dict.items():"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_13972691","line":285,"range":{"start_line":285,"start_character":22,"end_line":285,"end_character":23},"in_reply_to":"7a3c09a3_d4e6d042","updated":"2017-01-24 03:41:51.000000000","message":"Done","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        for vlan_tag, v in self.conj_ids.items():"},{"line_number":286,"context_line":"            update \u003d False"},{"line_number":287,"context_line":"            for di_et, dict_set in v.items():"},{"line_number":288,"context_line":"                for remote_sg_id, unused in unused_dict.items():"},{"line_number":289,"context_line":"                    if (remote_sg_id in dict_set and"},{"line_number":290,"context_line":"                        dict_set[remote_sg_id] \u0026 unused):"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_14ecd828","line":287,"updated":"2017-01-18 13:25:18.000000000","message":"nit: I\u0027d avoid contractions on direction/ethertype\n\nnit: Could you give a more specific name to dict_set ?","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        for vlan_tag, v in self.conj_ids.items():"},{"line_number":286,"context_line":"            update \u003d False"},{"line_number":287,"context_line":"            for di_et, dict_set in v.items():"},{"line_number":288,"context_line":"                for remote_sg_id, unused in unused_dict.items():"},{"line_number":289,"context_line":"                    if (remote_sg_id in dict_set and"},{"line_number":290,"context_line":"                        dict_set[remote_sg_id] \u0026 unused):"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_339262a0","line":287,"in_reply_to":"7a3c09a3_14ecd828","updated":"2017-01-24 03:41:51.000000000","message":"Done","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":443,"context_line":"        if not member_ips:"},{"line_number":444,"context_line":"            self.maybe_delete_sg(sg_id)"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def maybe_delete_sg(self, sg_id):"},{"line_number":447,"context_line":"        # This function must be called when the number of ports"},{"line_number":448,"context_line":"        # associated to sg_id drops to zero, as it isn\u0027t possible"},{"line_number":449,"context_line":"        # to know SG deletions from agents due to RPC API design."}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_e18a291a","line":446,"updated":"2017-01-18 13:25:18.000000000","message":"nit: I\u0027d change the function name, since this own\u0027t delete a sg, it will check if an sg needs to be marked for deletion.\n\nmark_sg_for_deletion_where_necessary(self, sg_id) ?\n\nI guess this function may be private\n_mark_sg_for_deletion_where_necessary(...)","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":443,"context_line":"        if not member_ips:"},{"line_number":444,"context_line":"            self.maybe_delete_sg(sg_id)"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def maybe_delete_sg(self, sg_id):"},{"line_number":447,"context_line":"        # This function must be called when the number of ports"},{"line_number":448,"context_line":"        # associated to sg_id drops to zero, as it isn\u0027t possible"},{"line_number":449,"context_line":"        # to know SG deletions from agents due to RPC API design."}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_53dc4e12","line":446,"in_reply_to":"7a3c09a3_e18a291a","updated":"2017-01-24 03:41:51.000000000","message":"Done","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":444,"context_line":"            self.maybe_delete_sg(sg_id)"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def maybe_delete_sg(self, sg_id):"},{"line_number":447,"context_line":"        # This function must be called when the number of ports"},{"line_number":448,"context_line":"        # associated to sg_id drops to zero, as it isn\u0027t possible"},{"line_number":449,"context_line":"        # to know SG deletions from agents due to RPC API design."},{"line_number":450,"context_line":"        sec_group \u003d self.sg_port_map.get_or_create_sg(sg_id)"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_94c7e894","line":447,"updated":"2017-01-18 13:25:18.000000000","message":"nit: This could be a docstring.","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":444,"context_line":"            self.maybe_delete_sg(sg_id)"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def maybe_delete_sg(self, sg_id):"},{"line_number":447,"context_line":"        # This function must be called when the number of ports"},{"line_number":448,"context_line":"        # associated to sg_id drops to zero, as it isn\u0027t possible"},{"line_number":449,"context_line":"        # to know SG deletions from agents due to RPC API design."},{"line_number":450,"context_line":"        sec_group \u003d self.sg_port_map.get_or_create_sg(sg_id)"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_93ddd616","line":447,"in_reply_to":"7a3c09a3_94c7e894","updated":"2017-01-24 03:41:51.000000000","message":"Done","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":825,"context_line":"                self._accept_flow(**flow)"},{"line_number":826,"context_line":""},{"line_number":827,"context_line":"        # Install action\u003dconjunction(conj_id, 2/2) flows,"},{"line_number":828,"context_line":"        # which depend on SG rules other than remote_group_id."},{"line_number":829,"context_line":"        for sec_group_id, rule in ("},{"line_number":830,"context_line":"                self._create_remote_rules_generator_for_port(port)):"},{"line_number":831,"context_line":"            direction \u003d rule[\u0027direction\u0027]"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_84e5f309","line":828,"updated":"2017-01-18 13:25:18.000000000","message":"more nits: can we put this on a separate function ? \n\nself._install_remote_conj_flows(....) or something like that?","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":825,"context_line":"                self._accept_flow(**flow)"},{"line_number":826,"context_line":""},{"line_number":827,"context_line":"        # Install action\u003dconjunction(conj_id, 2/2) flows,"},{"line_number":828,"context_line":"        # which depend on SG rules other than remote_group_id."},{"line_number":829,"context_line":"        for sec_group_id, rule in ("},{"line_number":830,"context_line":"                self._create_remote_rules_generator_for_port(port)):"},{"line_number":831,"context_line":"            direction \u003d rule[\u0027direction\u0027]"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_f6852807","line":828,"in_reply_to":"7a3c09a3_84e5f309","updated":"2017-01-24 03:41:51.000000000","message":"Done","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":873,"context_line":"            flows \u003d rules.create_flows_for_ip_address("},{"line_number":874,"context_line":"                ip_addr, direction, ethertype, vlan_tag, [0])"},{"line_number":875,"context_line":"            for f in flows:"},{"line_number":876,"context_line":"                # The following del statements are for complying the"},{"line_number":877,"context_line":"                # OpenFlow spec. It forbids the use of these fields in"},{"line_number":878,"context_line":"                # non-strict delete flow messages."},{"line_number":879,"context_line":"                del f[\u0027actions\u0027]"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_c4c29b52","line":876,"updated":"2017-01-18 13:25:18.000000000","message":"would it make sense to move L876-LL880 inside _delete_flows ?","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":873,"context_line":"            flows \u003d rules.create_flows_for_ip_address("},{"line_number":874,"context_line":"                ip_addr, direction, ethertype, vlan_tag, [0])"},{"line_number":875,"context_line":"            for f in flows:"},{"line_number":876,"context_line":"                # The following del statements are for complying the"},{"line_number":877,"context_line":"                # OpenFlow spec. It forbids the use of these fields in"},{"line_number":878,"context_line":"                # non-strict delete flow messages."},{"line_number":879,"context_line":"                del f[\u0027actions\u0027]"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_79192979","line":876,"in_reply_to":"7a3c09a3_c4c29b52","updated":"2017-01-24 03:41:51.000000000","message":"Maybe, but I don\u0027t think this is _delete_flows\u0027s job.\n\nThe code is somewhat abusing create_flows_for_ip_address to generate delete flow message.","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":186,"context_line":"class ConjIdMap(object):"},{"line_number":187,"context_line":"    def __init__(self):"},{"line_number":188,"context_line":"        self.id_map \u003d collections.defaultdict(self._conj_id_factory)"},{"line_number":189,"context_line":"        self.id_free \u003d collections.deque()"},{"line_number":190,"context_line":"        self.max_id \u003d 0"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    def _conj_id_factory(self):"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_8d8f5f22","line":189,"updated":"2017-04-18 12:40:26.000000000","message":"Given that conj_id is 32 bit number, is it necessary to reuse conj_ids?","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":186,"context_line":"class ConjIdMap(object):"},{"line_number":187,"context_line":"    def __init__(self):"},{"line_number":188,"context_line":"        self.id_map \u003d collections.defaultdict(self._conj_id_factory)"},{"line_number":189,"context_line":"        self.id_free \u003d collections.deque()"},{"line_number":190,"context_line":"        self.max_id \u003d 0"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    def _conj_id_factory(self):"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_3ad4ea1f","line":189,"in_reply_to":"7ffa3b31_8d8f5f22","updated":"2017-04-20 07:22:44.000000000","message":"We\u0027ll use them up in less than a year if 100 conj_ids are allocated per second.\nThat\u0027 quite unlikely but I\u0027d like to be conservative here.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        Allocate one if necessary.  The returned ID is always an even"},{"line_number":203,"context_line":"        number, allowing the caller to use 2 IDs for each combination."},{"line_number":204,"context_line":"        \"\"\""},{"line_number":205,"context_line":"        assert direction in [firewall.EGRESS_DIRECTION,"},{"line_number":206,"context_line":"                             firewall.INGRESS_DIRECTION]"},{"line_number":207,"context_line":"        assert ethertype in [lib_const.IPv4, lib_const.IPv6]"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_0d134f5e","line":205,"range":{"start_line":205,"start_character":8,"end_line":205,"end_character":15},"updated":"2017-04-18 12:40:26.000000000","message":"Can we get more meaningful messages here?","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        Allocate one if necessary.  The returned ID is always an even"},{"line_number":203,"context_line":"        number, allowing the caller to use 2 IDs for each combination."},{"line_number":204,"context_line":"        \"\"\""},{"line_number":205,"context_line":"        assert direction in [firewall.EGRESS_DIRECTION,"},{"line_number":206,"context_line":"                             firewall.INGRESS_DIRECTION]"},{"line_number":207,"context_line":"        assert ethertype in [lib_const.IPv4, lib_const.IPv6]"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_da9ce65e","line":205,"range":{"start_line":205,"start_character":8,"end_line":205,"end_character":15},"in_reply_to":"7ffa3b31_0d134f5e","updated":"2017-04-20 07:22:44.000000000","message":"These asserts are to avoid silly coding mistakes. I think programmers can get the meaning without extra messages.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"c31d5311f8a9a9a3469ccee705e36e1078f2c66b","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        Allocate one if necessary.  The returned ID is always an even"},{"line_number":203,"context_line":"        number, allowing the caller to use 2 IDs for each combination."},{"line_number":204,"context_line":"        \"\"\""},{"line_number":205,"context_line":"        assert direction in [firewall.EGRESS_DIRECTION,"},{"line_number":206,"context_line":"                             firewall.INGRESS_DIRECTION]"},{"line_number":207,"context_line":"        assert ethertype in [lib_const.IPv4, lib_const.IPv6]"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_dd70120b","line":205,"range":{"start_line":205,"start_character":8,"end_line":205,"end_character":15},"in_reply_to":"7ffa3b31_265825f8","updated":"2017-04-21 02:46:22.000000000","message":"Done","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"4120db89cb59793eac0320fcc0af315742309505","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        Allocate one if necessary.  The returned ID is always an even"},{"line_number":203,"context_line":"        number, allowing the caller to use 2 IDs for each combination."},{"line_number":204,"context_line":"        \"\"\""},{"line_number":205,"context_line":"        assert direction in [firewall.EGRESS_DIRECTION,"},{"line_number":206,"context_line":"                             firewall.INGRESS_DIRECTION]"},{"line_number":207,"context_line":"        assert ethertype in [lib_const.IPv4, lib_const.IPv6]"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_265825f8","line":205,"range":{"start_line":205,"start_character":8,"end_line":205,"end_character":15},"in_reply_to":"7ffa3b31_da9ce65e","updated":"2017-04-20 08:16:15.000000000","message":"It\u0027s usually operators who see the trace first.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        \"\"\""},{"line_number":205,"context_line":"        assert direction in [firewall.EGRESS_DIRECTION,"},{"line_number":206,"context_line":"                             firewall.INGRESS_DIRECTION]"},{"line_number":207,"context_line":"        assert ethertype in [lib_const.IPv4, lib_const.IPv6]"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        return self.id_map[(sg_id, remote_sg_id, direction, ethertype)]"},{"line_number":210,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_8d287f86","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":14},"updated":"2017-04-18 12:40:26.000000000","message":"Ditto","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":215,"context_line":"        \"\"\""},{"line_number":216,"context_line":"        result \u003d []"},{"line_number":217,"context_line":"        for k in list(self.id_map.keys()):"},{"line_number":218,"context_line":"            if sg_id in k[0:2]:"},{"line_number":219,"context_line":"                conj_id \u003d self.id_map.pop(k)"},{"line_number":220,"context_line":"                result.append((k[1], conj_id))"},{"line_number":221,"context_line":"                self.id_free.append(conj_id)"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_0df90fc7","line":218,"range":{"start_line":218,"start_character":26,"end_line":218,"end_character":27},"updated":"2017-04-18 12:40:26.000000000","message":"Nit: This is implicit","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":215,"context_line":"        \"\"\""},{"line_number":216,"context_line":"        result \u003d []"},{"line_number":217,"context_line":"        for k in list(self.id_map.keys()):"},{"line_number":218,"context_line":"            if sg_id in k[0:2]:"},{"line_number":219,"context_line":"                conj_id \u003d self.id_map.pop(k)"},{"line_number":220,"context_line":"                result.append((k[1], conj_id))"},{"line_number":221,"context_line":"                self.id_free.append(conj_id)"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_ba891acb","line":218,"range":{"start_line":218,"start_character":26,"end_line":218,"end_character":27},"in_reply_to":"7ffa3b31_0df90fc7","updated":"2017-04-20 07:22:44.000000000","message":"Done","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        #     self.x[vlan_tag][(direction, ethertype)]"},{"line_number":242,"context_line":"        self.conj_ids \u003d collections.defaultdict(dict)"},{"line_number":243,"context_line":"        self.flow_state \u003d collections.defaultdict("},{"line_number":244,"context_line":"            functools.partial(collections.defaultdict, dict))"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    def _build_addr_conj_id_map(self, ethertype, sg_conj_id_map):"},{"line_number":247,"context_line":"        \"\"\"Build a map of addr -\u003e list of conj_ids.\"\"\""}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_6dc1b3e4","line":244,"range":{"start_line":244,"start_character":12,"end_line":244,"end_character":30},"updated":"2017-04-18 12:40:26.000000000","message":"Nit: You don\u0027t need to import functools here:\n\n self.flow_state \u003d collections.defaultdict(lambda: collections.defaultdict(dict))","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        #     self.x[vlan_tag][(direction, ethertype)]"},{"line_number":242,"context_line":"        self.conj_ids \u003d collections.defaultdict(dict)"},{"line_number":243,"context_line":"        self.flow_state \u003d collections.defaultdict("},{"line_number":244,"context_line":"            functools.partial(collections.defaultdict, dict))"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    def _build_addr_conj_id_map(self, ethertype, sg_conj_id_map):"},{"line_number":247,"context_line":"        \"\"\"Build a map of addr -\u003e list of conj_ids.\"\"\""}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_ba77dad4","line":244,"range":{"start_line":244,"start_character":12,"end_line":244,"end_character":30},"in_reply_to":"7ffa3b31_6dc1b3e4","updated":"2017-04-20 07:22:44.000000000","message":"Done","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":495,"context_line":"                # sec_group is still in use"},{"line_number":496,"context_line":"                continue"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"            unused_conj_ids \u003d self.conj_id_map.delete_sg(sg_id)"},{"line_number":499,"context_line":"            self.conj_ip_manager.remove_bulk(unused_conj_ids)"},{"line_number":500,"context_line":"            self.sg_port_map.delete_sg(sg_id)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def filter_defer_apply_on(self):"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_f86903be","line":499,"range":{"start_line":498,"start_character":0,"end_line":499,"end_character":61},"updated":"2017-04-18 12:40:26.000000000","message":"Given that after calling to self.conj_id_map method we always need to call again to self.conj_ip_manager passing result from self.conj_id_map while we never use self.conj_id_map alone, I think the self.conj_id_map should be an internal thing of self.conj_ip_manager.\n\nThat way we move complexity regarding conj_id from firewall driver down to conj_ip_manager.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"39b927513728eea24007057747af54b9dda7683b","unresolved":false,"context_lines":[{"line_number":495,"context_line":"                # sec_group is still in use"},{"line_number":496,"context_line":"                continue"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"            unused_conj_ids \u003d self.conj_id_map.delete_sg(sg_id)"},{"line_number":499,"context_line":"            self.conj_ip_manager.remove_bulk(unused_conj_ids)"},{"line_number":500,"context_line":"            self.sg_port_map.delete_sg(sg_id)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def filter_defer_apply_on(self):"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_c620614b","line":499,"range":{"start_line":498,"start_character":0,"end_line":499,"end_character":61},"in_reply_to":"7ffa3b31_b5792b30","updated":"2017-04-20 08:19:05.000000000","message":"Oh I see what you did. I didn\u0027t mean to squash the classes but using ConjIdMap inside of ConjIPFlowManager using composition. So we would still have two classes but OVSFirewall won\u0027t know about ConjIdMap as it will be an internal thing to ConjIpFlowManager.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"c31d5311f8a9a9a3469ccee705e36e1078f2c66b","unresolved":false,"context_lines":[{"line_number":495,"context_line":"                # sec_group is still in use"},{"line_number":496,"context_line":"                continue"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"            unused_conj_ids \u003d self.conj_id_map.delete_sg(sg_id)"},{"line_number":499,"context_line":"            self.conj_ip_manager.remove_bulk(unused_conj_ids)"},{"line_number":500,"context_line":"            self.sg_port_map.delete_sg(sg_id)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def filter_defer_apply_on(self):"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_bd630642","line":499,"range":{"start_line":498,"start_character":0,"end_line":499,"end_character":61},"in_reply_to":"7ffa3b31_c620614b","updated":"2017-04-21 02:46:22.000000000","message":"Done. I think it\u0027s better now.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":495,"context_line":"                # sec_group is still in use"},{"line_number":496,"context_line":"                continue"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"            unused_conj_ids \u003d self.conj_id_map.delete_sg(sg_id)"},{"line_number":499,"context_line":"            self.conj_ip_manager.remove_bulk(unused_conj_ids)"},{"line_number":500,"context_line":"            self.sg_port_map.delete_sg(sg_id)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def filter_defer_apply_on(self):"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_b5792b30","line":499,"range":{"start_line":498,"start_character":0,"end_line":499,"end_character":61},"in_reply_to":"7ffa3b31_f86903be","updated":"2017-04-20 07:22:44.000000000","message":"Not quite sure that approach improves readability but changed accordingly.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d81f8c9c9499420d08d9388f141afefc48d7699a","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        self.raw_rules \u003d []"},{"line_number":81,"context_line":"        self.remote_rules \u003d []"},{"line_number":82,"context_line":"        self.members \u003d {}"},{"line_number":83,"context_line":"        self.members_on_flows \u003d {}"},{"line_number":84,"context_line":"        self.ports \u003d set()"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def update_rules(self, rules):"}],"source_content_type":"text/x-python","patch_set":31,"id":"5ff73747_dbae2a97","line":83,"range":{"start_line":83,"start_character":8,"end_line":83,"end_character":29},"updated":"2017-04-26 15:51:23.000000000","message":"This is unused","commit_id":"192bc5f1a8781111b6b0c2211f4421fafacb27c7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d81f8c9c9499420d08d9388f141afefc48d7699a","unresolved":false,"context_lines":[{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    def _build_addr_conj_id_map(self, ethertype, sg_conj_id_map):"},{"line_number":253,"context_line":"        \"\"\"Build a map of addr -\u003e list of conj_ids.\"\"\""},{"line_number":254,"context_line":"        addr_to_conj \u003d collections.defaultdict(list)"},{"line_number":255,"context_line":"        for remote_id, conj_id_set in sg_conj_id_map.items():"},{"line_number":256,"context_line":"            remote_group \u003d self.driver.sg_port_map.get_sg(remote_id)"},{"line_number":257,"context_line":"            if not remote_group:"}],"source_content_type":"text/x-python","patch_set":31,"id":"5ff73747_8146e820","line":254,"range":{"start_line":254,"start_character":8,"end_line":254,"end_character":20},"updated":"2017-04-26 15:51:23.000000000","message":"I like the renaming from last PS :)","commit_id":"192bc5f1a8781111b6b0c2211f4421fafacb27c7"}],"neutron/agent/linux/openvswitch_firewall/rules.py":[{"author":{"_account_id":15309,"name":"zhaobo","email":"bzhaojyathousandy@gmail.com","username":"ZhaoBo"},"change_message_id":"04911f2ff45c1178acd797531836a0e9457de2c5","unresolved":false,"context_lines":[{"line_number":172,"context_line":"    flow[\u0027ct_state\u0027] \u003d ovsfw_consts.OF_STATE_NEW_NOT_ESTABLISHED"},{"line_number":173,"context_line":"    if flow[\u0027table\u0027] \u003d\u003d ovs_consts.RULES_INGRESS_TABLE:"},{"line_number":174,"context_line":"        flow[\u0027actions\u0027] \u003d ("},{"line_number":175,"context_line":"            \u0027ct(commit,zone\u003dNXM_NX_REG{:d}[0..15]),{:s}\u0027.format("},{"line_number":176,"context_line":"                ovsfw_consts.REG_NET, flow[\u0027actions\u0027]))"},{"line_number":177,"context_line":"    result.append(flow)"},{"line_number":178,"context_line":"    return result"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba5da102_fccb4551","line":175,"updated":"2016-10-31 12:04:39.000000000","message":"I think you should be concerned about https://review.openstack.org/#/c/389312/ .\nFor the logic, there is no \u0027dl_type\u0027 contained, but here is a ct() action. It will hit the same issue like the above patch mentioned..:)","commit_id":"97226cc6ec9714401736b7c606905fa2d858aafa"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"722bbcdb01efaa4ad5bb7bba8ff2c41610d5348b","unresolved":false,"context_lines":[{"line_number":172,"context_line":"    flow[\u0027ct_state\u0027] \u003d ovsfw_consts.OF_STATE_NEW_NOT_ESTABLISHED"},{"line_number":173,"context_line":"    if flow[\u0027table\u0027] \u003d\u003d ovs_consts.RULES_INGRESS_TABLE:"},{"line_number":174,"context_line":"        flow[\u0027actions\u0027] \u003d ("},{"line_number":175,"context_line":"            \u0027ct(commit,zone\u003dNXM_NX_REG{:d}[0..15]),{:s}\u0027.format("},{"line_number":176,"context_line":"                ovsfw_consts.REG_NET, flow[\u0027actions\u0027]))"},{"line_number":177,"context_line":"    result.append(flow)"},{"line_number":178,"context_line":"    return result"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba5da102_a313560c","line":175,"in_reply_to":"ba5da102_fccb4551","updated":"2016-11-01 09:47:06.000000000","message":"good catch! thanks ;)","commit_id":"97226cc6ec9714401736b7c606905fa2d858aafa"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"5ebfd859b50c26fe08d651ffbe9a5485df44b727","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    flow_template \u003d {"},{"line_number":147,"context_line":"        \u0027priority\u0027: 70,"},{"line_number":148,"context_line":"        \u0027dl_type\u0027: ovsfw_consts.ethertype_to_dl_type_map[ethertype],"},{"line_number":149,"context_line":"        \u0027reg_net\u0027: vlan_tag,  # needed for project separation"},{"line_number":150,"context_line":"    }"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    ip_ver \u003d ip_lib.get_ip_version(ip_prefix)"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a6eadb0_c5886f25","line":149,"updated":"2016-12-15 06:56:09.000000000","message":"It seems we cannot have full isolation with this. Consider the following situation:\n\nProjects: demo1, demo2\nNetworks: p1 (owned by demo1), p2 (owned by demo2), pshared (shared, vlan_tag\u003d333); all of their subnets span on 10.2.3.0/24\nSGs: default SG only\n\ndemo1 boots VMs: vm1a on p1, vm1b on pshared\ndemo2 boots VMs: vm2a on p2, vm2b on pshared\n\nLet\u0027s assume both vm1a and vm2b have the same IP address, say 10.2.3.4. We have a problem as demo1\u0027s and demo2\u0027s default SGs both want reg6\u003d333,nw_src\u003d10.2.3.4 with different conjunction ID actions.\n\nThere are several ways to solve this, but this patch cannot handle this properly.\n\nIf these networks have disjoint IP address ranges, it\u0027s ok.","commit_id":"424ed6caf8c8b7c5134c09c2bd125d36b35aadd2"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"304bd132b5da3ec0eb345bea2e1c7cba509c0127","unresolved":false,"context_lines":[{"line_number":144,"context_line":"    ip_prefix \u003d str(netaddr.IPNetwork(ip_address).cidr)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    flow_template \u003d {"},{"line_number":147,"context_line":"        \u0027priority\u0027: 70,"},{"line_number":148,"context_line":"        \u0027dl_type\u0027: ovsfw_consts.ethertype_to_dl_type_map[ethertype],"},{"line_number":149,"context_line":"        \u0027reg_net\u0027: vlan_tag,  # needed for project separation"},{"line_number":150,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":19,"id":"da4df55a_1eccbe34","line":147,"updated":"2016-12-22 10:06:22.000000000","message":"why 70? can you please add a comment here","commit_id":"765f675c16854458b7ba60fcc1e264f72aae7701"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"5a243435bd07d166149b8db35340a4d4aed7fc35","unresolved":false,"context_lines":[{"line_number":144,"context_line":"    ip_prefix \u003d str(netaddr.IPNetwork(ip_address).cidr)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    flow_template \u003d {"},{"line_number":147,"context_line":"        \u0027priority\u0027: 70,"},{"line_number":148,"context_line":"        \u0027dl_type\u0027: ovsfw_consts.ethertype_to_dl_type_map[ethertype],"},{"line_number":149,"context_line":"        \u0027reg_net\u0027: vlan_tag,  # needed for project separation"},{"line_number":150,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_3b2e79a5","line":147,"in_reply_to":"da4df55a_1eccbe34","updated":"2017-01-05 05:34:28.000000000","message":"The ovsfw code uses priority 70 for flows generated from SG rules.\nMaybe it\u0027s better to document them in the devref.","commit_id":"765f675c16854458b7ba60fcc1e264f72aae7701"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    flow_template \u003d {"},{"line_number":147,"context_line":"        \u0027priority\u0027: 70,"},{"line_number":148,"context_line":"        \u0027dl_type\u0027: ovsfw_consts.ethertype_to_dl_type_map[ethertype],"},{"line_number":149,"context_line":"        \u0027reg_net\u0027: vlan_tag,  # needed for project separation"},{"line_number":150,"context_line":"    }"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    ip_ver \u003d ip_lib.get_ip_version(ip_prefix)"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_64cd6765","line":149,"updated":"2017-01-18 13:25:18.000000000","message":"nice I hadn\u0027t seen the reg_net replacement, smart :)","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":150,"context_line":"    }"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    ip_ver \u003d ip_lib.get_ip_version(ip_prefix)"},{"line_number":153,"context_line":"    if direction \u003d\u003d firewall.EGRESS_DIRECTION:"},{"line_number":154,"context_line":"        flow_template[\u0027table\u0027] \u003d ovs_consts.RULES_EGRESS_TABLE"},{"line_number":155,"context_line":"        if ip_ver \u003d\u003d n_consts.IP_VERSION_4:"},{"line_number":156,"context_line":"            flow_template[\"nw_dst\"] \u003d ip_prefix"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_c471bb0d","line":153,"updated":"2017-01-18 13:25:18.000000000","message":"mega_nit: we could have a separate function to calculate the openflow var for this:\n\n    flow_var \u003d self._get_flow_var_for_ipver_and_direction(ip_ver, direction)\n    flow_template[flow_var] \u003d ip_prefix","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":150,"context_line":"    }"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    ip_ver \u003d ip_lib.get_ip_version(ip_prefix)"},{"line_number":153,"context_line":"    if direction \u003d\u003d firewall.EGRESS_DIRECTION:"},{"line_number":154,"context_line":"        flow_template[\u0027table\u0027] \u003d ovs_consts.RULES_EGRESS_TABLE"},{"line_number":155,"context_line":"        if ip_ver \u003d\u003d n_consts.IP_VERSION_4:"},{"line_number":156,"context_line":"            flow_template[\"nw_dst\"] \u003d ip_prefix"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_e4bad7e0","line":153,"in_reply_to":"7a3c09a3_c471bb0d","updated":"2017-01-24 03:41:51.000000000","message":"Done","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"def substitute_conjunction_actions(flows, dimension, conj_ids):"},{"line_number":183,"context_line":"    CT_STATES \u003d ["},{"line_number":184,"context_line":"        ovsfw_consts.OF_STATE_ESTABLISHED_NOT_REPLY,"},{"line_number":185,"context_line":"        ovsfw_consts.OF_STATE_NEW_NOT_ESTABLISHED]"},{"line_number":186,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_5e79ff90","line":183,"range":{"start_line":183,"start_character":4,"end_line":183,"end_character":13},"updated":"2017-04-18 12:40:26.000000000","message":"This should be a global constant, no need to create it per function call.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"def substitute_conjunction_actions(flows, dimension, conj_ids):"},{"line_number":183,"context_line":"    CT_STATES \u003d ["},{"line_number":184,"context_line":"        ovsfw_consts.OF_STATE_ESTABLISHED_NOT_REPLY,"},{"line_number":185,"context_line":"        ovsfw_consts.OF_STATE_NEW_NOT_ESTABLISHED]"},{"line_number":186,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_0ba55bb5","line":183,"range":{"start_line":183,"start_character":4,"end_line":183,"end_character":13},"in_reply_to":"7ffa3b31_5e79ff90","updated":"2017-04-20 07:22:44.000000000","message":"Done","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"}],"neutron/db/securitygroups_rpc_base.py":[{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"c55e1033f8a12b183a4f21eb10ebda96e7411197","unresolved":false,"context_lines":[{"line_number":194,"context_line":"            rule_dict \u003d {"},{"line_number":195,"context_line":"                \u0027direction\u0027: direction,"},{"line_number":196,"context_line":"                \u0027ethertype\u0027: ethertype,"},{"line_number":197,"context_line":"                \u0027id\u0027: rule_in_db[\u0027id\u0027]}"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"            for key in (\u0027protocol\u0027, \u0027port_range_min\u0027, \u0027port_range_max\u0027,"},{"line_number":200,"context_line":"                        \u0027remote_ip_prefix\u0027, \u0027remote_group_id\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_df814491","line":197,"updated":"2016-06-28 14:46:07.000000000","message":"Can we find a way to avoid sending rule id?\n\nThere are good reason to think of it:  upgradeability, since the newer agent will require rule_id the server is not sending, and otherwise fail.\n\nOtherwise you will need to include a fallback in agent to work without rule_id btw.","commit_id":"d6ba09ff7fd1fb3f7fe63137050f31c3c3d4f734"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"2f4aa64986d4137aa9c830d37e03b55e063bae62","unresolved":false,"context_lines":[{"line_number":194,"context_line":"            rule_dict \u003d {"},{"line_number":195,"context_line":"                \u0027direction\u0027: direction,"},{"line_number":196,"context_line":"                \u0027ethertype\u0027: ethertype,"},{"line_number":197,"context_line":"                \u0027id\u0027: rule_in_db[\u0027id\u0027]}"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"            for key in (\u0027protocol\u0027, \u0027port_range_min\u0027, \u0027port_range_max\u0027,"},{"line_number":200,"context_line":"                        \u0027remote_ip_prefix\u0027, \u0027remote_group_id\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad45d7e_e9ac63ef","line":197,"in_reply_to":"3aaa91ec_cf77ba39","updated":"2016-08-09 04:34:45.000000000","message":"Removed use of rule_ids in PS4.","commit_id":"d6ba09ff7fd1fb3f7fe63137050f31c3c3d4f734"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"a702d04ee1b72f0e0f7695f61c51a000331b021f","unresolved":false,"context_lines":[{"line_number":194,"context_line":"            rule_dict \u003d {"},{"line_number":195,"context_line":"                \u0027direction\u0027: direction,"},{"line_number":196,"context_line":"                \u0027ethertype\u0027: ethertype,"},{"line_number":197,"context_line":"                \u0027id\u0027: rule_in_db[\u0027id\u0027]}"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"            for key in (\u0027protocol\u0027, \u0027port_range_min\u0027, \u0027port_range_max\u0027,"},{"line_number":200,"context_line":"                        \u0027remote_ip_prefix\u0027, \u0027remote_group_id\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_cf77ba39","line":197,"in_reply_to":"3aaa91ec_df814491","updated":"2016-06-29 01:42:09.000000000","message":"I haven\u0027t considered upgradability or RPC versioning yet.\n\nWe can do without rule ID from a neutron-server and build rule ID equivalent on agent side, while that\u0027s a bit more complicated and less efficient.","commit_id":"d6ba09ff7fd1fb3f7fe63137050f31c3c3d4f734"}],"neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            id_ \u003d self.conj_id_map.get_conj_id("},{"line_number":232,"context_line":"                \u0027sg\u0027, \u0027remote\u0027, direction, constants.IPv4)"},{"line_number":233,"context_line":"            allocated.add(id_)"},{"line_number":234,"context_line":"        self.assertEqual(len(allocated), 2)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def test_get_conj_id_invalid(self):"},{"line_number":237,"context_line":"        self.assertRaises(AssertionError, self.conj_id_map.get_conj_id,"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_ed45c316","line":234,"range":{"start_line":234,"start_character":25,"end_line":234,"end_character":39},"updated":"2017-04-18 12:40:26.000000000","message":"This tests that for loop iterates over 2 values but doesn\u0027t validate an internal state of self.conj_id_map. You probably want to assert that self.conj_id_map.id_map contains 2 items.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            id_ \u003d self.conj_id_map.get_conj_id("},{"line_number":232,"context_line":"                \u0027sg\u0027, \u0027remote\u0027, direction, constants.IPv4)"},{"line_number":233,"context_line":"            allocated.add(id_)"},{"line_number":234,"context_line":"        self.assertEqual(len(allocated), 2)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def test_get_conj_id_invalid(self):"},{"line_number":237,"context_line":"        self.assertRaises(AssertionError, self.conj_id_map.get_conj_id,"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_2b637744","line":234,"range":{"start_line":234,"start_character":25,"end_line":234,"end_character":39},"in_reply_to":"7ffa3b31_ed45c316","updated":"2017-04-20 07:22:44.000000000","message":"Done","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":232,"context_line":"                \u0027sg\u0027, \u0027remote\u0027, direction, constants.IPv4)"},{"line_number":233,"context_line":"            allocated.add(id_)"},{"line_number":234,"context_line":"        self.assertEqual(len(allocated), 2)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def test_get_conj_id_invalid(self):"},{"line_number":237,"context_line":"        self.assertRaises(AssertionError, self.conj_id_map.get_conj_id,"},{"line_number":238,"context_line":"                          \u0027sg\u0027, \u0027remote\u0027, \u0027invalid-direction\u0027,"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_cd6d6799","line":235,"updated":"2017-04-18 12:40:26.000000000","message":"There should be a test that validates that calling get_conj_id with same parameters return the same conj_id.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":232,"context_line":"                \u0027sg\u0027, \u0027remote\u0027, direction, constants.IPv4)"},{"line_number":233,"context_line":"            allocated.add(id_)"},{"line_number":234,"context_line":"        self.assertEqual(len(allocated), 2)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def test_get_conj_id_invalid(self):"},{"line_number":237,"context_line":"        self.assertRaises(AssertionError, self.conj_id_map.get_conj_id,"},{"line_number":238,"context_line":"                          \u0027sg\u0027, \u0027remote\u0027, \u0027invalid-direction\u0027,"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_4bf9234b","line":235,"in_reply_to":"7ffa3b31_cd6d6799","updated":"2017-04-20 07:22:44.000000000","message":"Done","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        result \u003d self.conj_id_map.delete_sg(\u0027sg1\u0027)"},{"line_number":251,"context_line":"        self.assertIn((\u0027sg1\u0027, ids[0]), result)"},{"line_number":252,"context_line":"        self.assertIn((\u0027sg2\u0027, ids[1]), result)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"class TestConjIPFlowManager(base.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_edca636c","line":252,"updated":"2017-04-18 12:40:26.000000000","message":"Missing asserts that validate sg1 and sg2 are not longer part of self.conj_id_map.id_map","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        result \u003d self.conj_id_map.delete_sg(\u0027sg1\u0027)"},{"line_number":251,"context_line":"        self.assertIn((\u0027sg1\u0027, ids[0]), result)"},{"line_number":252,"context_line":"        self.assertIn((\u0027sg2\u0027, ids[1]), result)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"class TestConjIPFlowManager(base.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_6e4bcd58","line":252,"in_reply_to":"7ffa3b31_edca636c","updated":"2017-04-20 07:22:44.000000000","message":"Done","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":250,"context_line":"        result \u003d self.conj_id_map.delete_sg(\u0027sg1\u0027)"},{"line_number":251,"context_line":"        self.assertIn((\u0027sg1\u0027, ids[0]), result)"},{"line_number":252,"context_line":"        self.assertIn((\u0027sg2\u0027, ids[1]), result)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"class TestConjIPFlowManager(base.BaseTestCase):"},{"line_number":256,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_ed8f032c","line":253,"updated":"2017-04-18 12:40:26.000000000","message":"Missing test that validates removing group from id_map puts back allocated conj_id to be reused.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":250,"context_line":"        result \u003d self.conj_id_map.delete_sg(\u0027sg1\u0027)"},{"line_number":251,"context_line":"        self.assertIn((\u0027sg1\u0027, ids[0]), result)"},{"line_number":252,"context_line":"        self.assertIn((\u0027sg2\u0027, ids[1]), result)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"class TestConjIPFlowManager(base.BaseTestCase):"},{"line_number":256,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_2e85c55a","line":253,"in_reply_to":"7ffa3b31_ed8f032c","updated":"2017-04-20 07:22:44.000000000","message":"Done","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        self.conj_id \u003d 10"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"    def test_update_flows_for_vlan(self):"},{"line_number":264,"context_line":"        remote_group \u003d mock.Mock()"},{"line_number":265,"context_line":"        self.driver.sg_port_map.get_sg.return_value \u003d remote_group"},{"line_number":266,"context_line":"        remote_group.get_ethertype_filtered_addresses.return_value \u003d ["},{"line_number":267,"context_line":"            \u002710.22.3.4\u0027]"},{"line_number":268,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_d36e2895","line":265,"range":{"start_line":264,"start_character":0,"end_line":265,"end_character":66},"updated":"2017-04-18 12:40:26.000000000","message":"This can be done by a single line:\n\n remote_group \u003d self.driver.sg_port_map.get_sg.return_value","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        self.conj_id \u003d 10"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"    def test_update_flows_for_vlan(self):"},{"line_number":264,"context_line":"        remote_group \u003d mock.Mock()"},{"line_number":265,"context_line":"        self.driver.sg_port_map.get_sg.return_value \u003d remote_group"},{"line_number":266,"context_line":"        remote_group.get_ethertype_filtered_addresses.return_value \u003d ["},{"line_number":267,"context_line":"            \u002710.22.3.4\u0027]"},{"line_number":268,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_4e693189","line":265,"range":{"start_line":264,"start_character":0,"end_line":265,"end_character":66},"in_reply_to":"7ffa3b31_d36e2895","updated":"2017-04-20 07:22:44.000000000","message":"Done","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d81f8c9c9499420d08d9388f141afefc48d7699a","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        self.conj_id_map \u003d ovsfw.ConjIdMap()"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def test_get_conj_id(self):"},{"line_number":230,"context_line":"        allocated \u003d []"},{"line_number":231,"context_line":"        for direction in [firewall.EGRESS_DIRECTION,"},{"line_number":232,"context_line":"                          firewall.INGRESS_DIRECTION]:"},{"line_number":233,"context_line":"            id_ \u003d self.conj_id_map.get_conj_id("}],"source_content_type":"text/x-python","patch_set":31,"id":"5ff73747_61f19ccd","line":230,"range":{"start_line":230,"start_character":0,"end_line":230,"end_character":22},"updated":"2017-04-26 15:51:23.000000000","message":"I don\u0027t think this is needed as it\u0027s not used internally by the prod code.","commit_id":"192bc5f1a8781111b6b0c2211f4421fafacb27c7"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"54e29239dfdd17d066c8a41230ba0eb82f2c38a3","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        self.conj_id_map \u003d ovsfw.ConjIdMap()"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def test_get_conj_id(self):"},{"line_number":230,"context_line":"        allocated \u003d []"},{"line_number":231,"context_line":"        for direction in [firewall.EGRESS_DIRECTION,"},{"line_number":232,"context_line":"                          firewall.INGRESS_DIRECTION]:"},{"line_number":233,"context_line":"            id_ \u003d self.conj_id_map.get_conj_id("}],"source_content_type":"text/x-python","patch_set":31,"id":"5ff73747_f6f68163","line":230,"range":{"start_line":230,"start_character":0,"end_line":230,"end_character":22},"in_reply_to":"5ff73747_61f19ccd","updated":"2017-04-27 04:17:00.000000000","message":"This test is to confirm that get_conj_id\n\n1) returns different ids for different set of args\n2) returns the same id if the args are the same.\n\nI couldn\u0027t understand \"it\u0027s not used internally...\" part. Could you describe a bit more?","commit_id":"192bc5f1a8781111b6b0c2211f4421fafacb27c7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ce8f5ba1c19bc15b195bf81dc5c05fd77e5080f9","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        self.conj_id_map \u003d ovsfw.ConjIdMap()"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def test_get_conj_id(self):"},{"line_number":230,"context_line":"        allocated \u003d []"},{"line_number":231,"context_line":"        for direction in [firewall.EGRESS_DIRECTION,"},{"line_number":232,"context_line":"                          firewall.INGRESS_DIRECTION]:"},{"line_number":233,"context_line":"            id_ \u003d self.conj_id_map.get_conj_id("}],"source_content_type":"text/x-python","patch_set":31,"id":"5ff73747_6c261e18","line":230,"range":{"start_line":230,"start_character":0,"end_line":230,"end_character":22},"in_reply_to":"5ff73747_f6f68163","updated":"2017-04-27 06:14:18.000000000","message":"oh, I originally thought that it just tests that two items are in the list but I missed it\u0027s made a set first to ensure uniqueness.","commit_id":"192bc5f1a8781111b6b0c2211f4421fafacb27c7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d81f8c9c9499420d08d9388f141afefc48d7699a","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        result \u003d self.conj_id_map.delete_sg(\u0027sg1\u0027)"},{"line_number":257,"context_line":"        self.assertIn((\u0027sg1\u0027, ids[0]), result)"},{"line_number":258,"context_line":"        self.assertIn((\u0027sg2\u0027, ids[1]), result)"},{"line_number":259,"context_line":"        self.assertEqual(len(self.conj_id_map.id_map), 0)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        reallocated \u003d self.conj_id_map.get_conj_id("},{"line_number":262,"context_line":"            \u0027sg-foo\u0027, \u0027sg-foo\u0027, firewall.INGRESS_DIRECTION,"}],"source_content_type":"text/x-python","patch_set":31,"id":"5ff73747_e1ab6c86","line":259,"range":{"start_line":259,"start_character":13,"end_line":259,"end_character":24},"updated":"2017-04-26 15:51:23.000000000","message":"Nit: Just assertFalse(self.conj_id_map.id_map) would suffice.","commit_id":"192bc5f1a8781111b6b0c2211f4421fafacb27c7"}],"neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py":[{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dffc6f937fed75ada11fba6be69ea4e5fd019e96","unresolved":false,"context_lines":[{"line_number":379,"context_line":"            del f[\u0027actions\u0027]"},{"line_number":380,"context_line":"            del f[\u0027ct_state\u0027]"},{"line_number":381,"context_line":"            self.assertEqual(expected_template, f)"},{"line_number":382,"context_line":"            expected_template[\u0027conj_id\u0027] +\u003d 1"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_57981f78","line":382,"updated":"2017-01-18 13:25:18.000000000","message":"I see no tests for the delete/cleanup behaviours \n\nI believe ConjIdMap and ConjIPFlowManager classes would benefit from some unit testing themselves.","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f03d728e7e17487f7a89eb837987a2950aa6c57a","unresolved":false,"context_lines":[{"line_number":379,"context_line":"            del f[\u0027actions\u0027]"},{"line_number":380,"context_line":"            del f[\u0027ct_state\u0027]"},{"line_number":381,"context_line":"            self.assertEqual(expected_template, f)"},{"line_number":382,"context_line":"            expected_template[\u0027conj_id\u0027] +\u003d 1"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a3905b3_cc3d74c5","line":382,"in_reply_to":"7a3c09a3_57981f78","updated":"2017-01-24 03:41:51.000000000","message":"Sure.\n\nAdded some tests in test_firewall.","commit_id":"6f82cde78d89c424ac1b0131b94d73fecf5b0fe3"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6f51145fef8aa7e7250f4da6656b836390eaa810","unresolved":false,"context_lines":[{"line_number":346,"context_line":"class TestCreateConjFlows(base.BaseTestCase):"},{"line_number":347,"context_line":"    def test_create_conj_flows(self):"},{"line_number":348,"context_line":"        ovs_port \u003d mock.Mock(vif_mac\u003d\u002700:00:00:00:00:00\u0027)"},{"line_number":349,"context_line":"        ovs_port.ofport \u003d 1"},{"line_number":350,"context_line":"        port_dict \u003d {\u0027device\u0027: \u0027port_id\u0027}"},{"line_number":351,"context_line":"        port \u003d ovsfw.OFPort("},{"line_number":352,"context_line":"            port_dict, ovs_port, vlan_tag\u003dTESTING_VLAN_TAG)"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_fe4a0b25","line":349,"range":{"start_line":349,"start_character":0,"end_line":349,"end_character":27},"updated":"2017-04-18 12:40:26.000000000","message":"Nit: This can be passed to Mock() directly.","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"cba473407b13299914f6597d856d735dadef73d8","unresolved":false,"context_lines":[{"line_number":346,"context_line":"class TestCreateConjFlows(base.BaseTestCase):"},{"line_number":347,"context_line":"    def test_create_conj_flows(self):"},{"line_number":348,"context_line":"        ovs_port \u003d mock.Mock(vif_mac\u003d\u002700:00:00:00:00:00\u0027)"},{"line_number":349,"context_line":"        ovs_port.ofport \u003d 1"},{"line_number":350,"context_line":"        port_dict \u003d {\u0027device\u0027: \u0027port_id\u0027}"},{"line_number":351,"context_line":"        port \u003d ovsfw.OFPort("},{"line_number":352,"context_line":"            port_dict, ovs_port, vlan_tag\u003dTESTING_VLAN_TAG)"}],"source_content_type":"text/x-python","patch_set":29,"id":"7ffa3b31_2b4fd36b","line":349,"range":{"start_line":349,"start_character":0,"end_line":349,"end_character":27},"in_reply_to":"7ffa3b31_fe4a0b25","updated":"2017-04-20 07:22:44.000000000","message":"Done","commit_id":"e6781dc31f9eb3f8214a1e609db8694b7610bdd9"}]}