)]}'
{"neutron/agent/l3/extensions/conntrack_helper.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":"DEFAULT_CONNTRACK_HELPER_CHAIN \u003d \u0027cth\u0027"},{"line_number":32,"context_line":"CONNTRACK_HELPER_PREFIX \u003d \u0027cthelper-\u0027"},{"line_number":33,"context_line":"CONNTRACK_HELPER_CHAIN_PREFIX \u003d \u0027cth-\u0027"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class ConntrackHelperMapping(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_87d4fcbe","line":33,"range":{"start_line":33,"start_character":33,"end_line":33,"end_character":36},"updated":"2019-04-24 15:43:10.000000000","message":"small nit: if you have DEFAULT_CONNTRACK_HELPER_CHAIN, I suggest:\n\nCONNTRACK_HELPER_CHAIN_PREFIX \u003d DEFAULT_CONNTRACK_HELPER_CHAIN + \u0027-\u0027","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":"DEFAULT_CONNTRACK_HELPER_CHAIN \u003d \u0027cth\u0027"},{"line_number":32,"context_line":"CONNTRACK_HELPER_PREFIX \u003d \u0027cthelper-\u0027"},{"line_number":33,"context_line":"CONNTRACK_HELPER_CHAIN_PREFIX \u003d \u0027cth-\u0027"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class ConntrackHelperMapping(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_b1a494a7","line":33,"range":{"start_line":33,"start_character":33,"end_line":33,"end_character":36},"in_reply_to":"ffb9cba7_87d4fcbe","updated":"2019-04-29 11:28:39.000000000","message":"Done","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":36,"context_line":"class ConntrackHelperMapping(object):"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def __init__(self):"},{"line_number":39,"context_line":"        self.managed_conntrack_helpers \u003d {}"},{"line_number":40,"context_line":"        \"\"\""},{"line_number":41,"context_line":"        router_conntrack_helper_mapping \u003d {"},{"line_number":42,"context_line":"           router_id_1: set(cth_id_1, cth_id_2),"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_195074d4","line":39,"updated":"2019-04-24 15:43:10.000000000","message":"small nit: because both dicts (managed_conntrack_helpers and router_conntrack_helper_mapping) must be synchronized, as is done in this class, we should prevent anyone from accessing to those dicts outside this class. I suggest to make both private.","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def update_conntrack_helpers(self, conntrack_helpers):"},{"line_number":54,"context_line":"        for cth in conntrack_helpers:"},{"line_number":55,"context_line":"            self.managed_conntrack_helpers[cth.id] \u003d cth"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def get_conntack_helper(self, conntrack_helper_id):"},{"line_number":58,"context_line":"        return self.managed_conntrack_helpers.get(conntrack_helper_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_99cf84af","line":55,"updated":"2019-04-24 15:43:10.000000000","message":"What is happening if this \"cth\" doesn\u0027t exist in the class?\nself.managed_conntrack_helpers will be populated but self.router_conntrack_helper_mapping won\u0027t have this value.","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def update_conntrack_helpers(self, conntrack_helpers):"},{"line_number":54,"context_line":"        for cth in conntrack_helpers:"},{"line_number":55,"context_line":"            self.managed_conntrack_helpers[cth.id] \u003d cth"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def get_conntack_helper(self, conntrack_helper_id):"},{"line_number":58,"context_line":"        return self.managed_conntrack_helpers.get(conntrack_helper_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_828d7cd4","line":55,"in_reply_to":"ffb9cba7_99cf84af","updated":"2019-04-29 11:28:39.000000000","message":"Since it\u0027s an update it should be there.\nI added a conditin that checks and adds it to self.router_conntrack_helper_mapping just in case.","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":63,"context_line":"                continue"},{"line_number":64,"context_line":"            del self.managed_conntrack_helpers[cth.id]"},{"line_number":65,"context_line":"            self.router_conntrack_helper_mapping[cth.router_id].remove(cth.id)"},{"line_number":66,"context_line":"            if len(self.router_conntrack_helper_mapping[cth.router_id]) \u003d\u003d 0:"},{"line_number":67,"context_line":"                del self.router_conntrack_helper_mapping[cth.router_id]"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def clear_by_router_id(self, router_id):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_995d44eb","line":66,"updated":"2019-04-24 15:43:10.000000000","message":"nit: if set will is empty, then\n\nif self.router_conntrack_helper_mapping[cth.router_id]:","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":63,"context_line":"                continue"},{"line_number":64,"context_line":"            del self.managed_conntrack_helpers[cth.id]"},{"line_number":65,"context_line":"            self.router_conntrack_helper_mapping[cth.router_id].remove(cth.id)"},{"line_number":66,"context_line":"            if len(self.router_conntrack_helper_mapping[cth.router_id]) \u003d\u003d 0:"},{"line_number":67,"context_line":"                del self.router_conntrack_helper_mapping[cth.router_id]"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def clear_by_router_id(self, router_id):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_5d8f6974","line":66,"in_reply_to":"ffb9cba7_995d44eb","updated":"2019-04-29 11:28:39.000000000","message":"Done","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":66,"context_line":"            if len(self.router_conntrack_helper_mapping[cth.router_id]) \u003d\u003d 0:"},{"line_number":67,"context_line":"                del self.router_conntrack_helper_mapping[cth.router_id]"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def clear_by_router_id(self, router_id):"},{"line_number":70,"context_line":"        router_cth_ids \u003d self.router_conntrack_helper_mapping.get(router_id)"},{"line_number":71,"context_line":"        if not router_cth_ids:"},{"line_number":72,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_fc91ce9a","line":69,"updated":"2019-04-24 15:43:10.000000000","message":"I don\u0027t understand the result of this method: we should remove \"router_id\" from \"router_conntrack_helper_mapping\" always. That means, all \"cth\" from \"router_cht_ids\" should be contained in \"managed_conntrack_helpers\"","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":66,"context_line":"            if len(self.router_conntrack_helper_mapping[cth.router_id]) \u003d\u003d 0:"},{"line_number":67,"context_line":"                del self.router_conntrack_helper_mapping[cth.router_id]"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def clear_by_router_id(self, router_id):"},{"line_number":70,"context_line":"        router_cth_ids \u003d self.router_conntrack_helper_mapping.get(router_id)"},{"line_number":71,"context_line":"        if not router_cth_ids:"},{"line_number":72,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_7d8cad7e","line":69,"in_reply_to":"ffb9cba7_fc91ce9a","updated":"2019-04-29 11:28:39.000000000","message":"Right, I simplified this method. i.e we can assume a the data in router_conntrack_helper_mapping and managed_conntrack_helpers are in sync.","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        router_cth_ids \u003d self.router_conntrack_helper_mapping.get(router_id)"},{"line_number":71,"context_line":"        if not router_cth_ids:"},{"line_number":72,"context_line":"            return"},{"line_number":73,"context_line":"        for cth_id in list(router_cth_ids):"},{"line_number":74,"context_line":"            if not self.get_conntack_helper(cth_id):"},{"line_number":75,"context_line":"                continue"},{"line_number":76,"context_line":"            del self.managed_conntrack_helpers[cth_id]"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_19ecf4f1","line":73,"range":{"start_line":73,"start_character":22,"end_line":73,"end_character":42},"updated":"2019-04-24 15:43:10.000000000","message":"\"router_cth_ids\" is a set and can be accessed directly in this generator:\n\n  for cth_id in router_cth_ids:","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"8c3d0def62caf5a94c95fe93b1c2ea0cf66a8883","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        router_cth_ids \u003d self.router_conntrack_helper_mapping.get(router_id)"},{"line_number":71,"context_line":"        if not router_cth_ids:"},{"line_number":72,"context_line":"            return"},{"line_number":73,"context_line":"        for cth_id in list(router_cth_ids):"},{"line_number":74,"context_line":"            if not self.get_conntack_helper(cth_id):"},{"line_number":75,"context_line":"                continue"},{"line_number":76,"context_line":"            del self.managed_conntrack_helpers[cth_id]"}],"source_content_type":"text/x-python","patch_set":8,"id":"dfbec78f_c4e98b1a","line":73,"range":{"start_line":73,"start_character":22,"end_line":73,"end_character":42},"in_reply_to":"dfbec78f_ac4b6bae","updated":"2019-05-09 06:04:18.000000000","message":"Ah, thanks, you are correct! Prior to the last update I did a remove inside the loop:\n\n    self._router_conntrack_helper_mapping[router_id].remove(cth_id)\n\nWith the last update I delete the router_id from the mapping after iterating:\n\n    del self._router_conntrack_helper_mapping[router_id]\n\nSo the list() is no longer needed.","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"8c3d0def62caf5a94c95fe93b1c2ea0cf66a8883","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        router_cth_ids \u003d self.router_conntrack_helper_mapping.get(router_id)"},{"line_number":71,"context_line":"        if not router_cth_ids:"},{"line_number":72,"context_line":"            return"},{"line_number":73,"context_line":"        for cth_id in list(router_cth_ids):"},{"line_number":74,"context_line":"            if not self.get_conntack_helper(cth_id):"},{"line_number":75,"context_line":"                continue"},{"line_number":76,"context_line":"            del self.managed_conntrack_helpers[cth_id]"}],"source_content_type":"text/x-python","patch_set":8,"id":"dfbec78f_64aa1fd3","line":73,"range":{"start_line":73,"start_character":22,"end_line":73,"end_character":42},"in_reply_to":"dfbec78f_ac4b6bae","updated":"2019-05-09 06:04:18.000000000","message":"You are correct. I","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        router_cth_ids \u003d self.router_conntrack_helper_mapping.get(router_id)"},{"line_number":71,"context_line":"        if not router_cth_ids:"},{"line_number":72,"context_line":"            return"},{"line_number":73,"context_line":"        for cth_id in list(router_cth_ids):"},{"line_number":74,"context_line":"            if not self.get_conntack_helper(cth_id):"},{"line_number":75,"context_line":"                continue"},{"line_number":76,"context_line":"            del self.managed_conntrack_helpers[cth_id]"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_a261a007","line":73,"range":{"start_line":73,"start_character":22,"end_line":73,"end_character":42},"in_reply_to":"ffb9cba7_19ecf4f1","updated":"2019-04-29 11:28:39.000000000","message":"Using the list() to avoid:\n  RuntimeError: Set changed size during iteration","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c9928f2e8a1fa45e3e5cc4b556c092916717dd11","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        router_cth_ids \u003d self.router_conntrack_helper_mapping.get(router_id)"},{"line_number":71,"context_line":"        if not router_cth_ids:"},{"line_number":72,"context_line":"            return"},{"line_number":73,"context_line":"        for cth_id in list(router_cth_ids):"},{"line_number":74,"context_line":"            if not self.get_conntack_helper(cth_id):"},{"line_number":75,"context_line":"                continue"},{"line_number":76,"context_line":"            del self.managed_conntrack_helpers[cth_id]"}],"source_content_type":"text/x-python","patch_set":8,"id":"dfbec78f_ac4b6bae","line":73,"range":{"start_line":73,"start_character":22,"end_line":73,"end_character":42},"in_reply_to":"ffb9cba7_a261a007","updated":"2019-05-02 16:25:10.000000000","message":"Hmmm this should not happen. When do you expect to have this problem?","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        if not router_cth_ids:"},{"line_number":72,"context_line":"            return"},{"line_number":73,"context_line":"        for cth_id in list(router_cth_ids):"},{"line_number":74,"context_line":"            if not self.get_conntack_helper(cth_id):"},{"line_number":75,"context_line":"                continue"},{"line_number":76,"context_line":"            del self.managed_conntrack_helpers[cth_id]"},{"line_number":77,"context_line":"            self.router_conntrack_helper_mapping[router_id].remove(cth_id)"},{"line_number":78,"context_line":"        if len(self.router_conntrack_helper_mapping.get(router_id)) \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_59cb4c71","line":75,"range":{"start_line":74,"start_character":12,"end_line":75,"end_character":24},"updated":"2019-04-24 15:43:10.000000000","message":"How can this be possible? It shouldn\u0027t","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        if not router_cth_ids:"},{"line_number":72,"context_line":"            return"},{"line_number":73,"context_line":"        for cth_id in list(router_cth_ids):"},{"line_number":74,"context_line":"            if not self.get_conntack_helper(cth_id):"},{"line_number":75,"context_line":"                continue"},{"line_number":76,"context_line":"            del self.managed_conntrack_helpers[cth_id]"},{"line_number":77,"context_line":"            self.router_conntrack_helper_mapping[router_id].remove(cth_id)"},{"line_number":78,"context_line":"        if len(self.router_conntrack_helper_mapping.get(router_id)) \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_82549c22","line":75,"range":{"start_line":74,"start_character":12,"end_line":75,"end_character":24},"in_reply_to":"ffb9cba7_59cb4c71","updated":"2019-04-29 11:28:39.000000000","message":"right, I removed this code.","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":125,"context_line":"    def _install_default_rules(self, iptables_manager, version):"},{"line_number":126,"context_line":"        default_rule \u003d \u0027-j %s-%s\u0027 % (iptables_manager.wrap_name,"},{"line_number":127,"context_line":"                                     DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":128,"context_line":"        if version \u003d\u003d 4:"},{"line_number":129,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_chain("},{"line_number":130,"context_line":"                DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":131,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_rule(\u0027PREROUTING\u0027, default_rule)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_48c6109d","line":128,"range":{"start_line":128,"start_character":22,"end_line":128,"end_character":23},"updated":"2019-04-24 15:43:10.000000000","message":"Please, use from neutron_lib.constants.IPv4","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":125,"context_line":"    def _install_default_rules(self, iptables_manager, version):"},{"line_number":126,"context_line":"        default_rule \u003d \u0027-j %s-%s\u0027 % (iptables_manager.wrap_name,"},{"line_number":127,"context_line":"                                     DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":128,"context_line":"        if version \u003d\u003d 4:"},{"line_number":129,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_chain("},{"line_number":130,"context_line":"                DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":131,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_rule(\u0027PREROUTING\u0027, default_rule)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_b1057448","line":128,"range":{"start_line":128,"start_character":22,"end_line":128,"end_character":23},"in_reply_to":"ffb9cba7_48c6109d","updated":"2019-04-29 11:28:39.000000000","message":"Done","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_chain("},{"line_number":130,"context_line":"                DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":131,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_rule(\u0027PREROUTING\u0027, default_rule)"},{"line_number":132,"context_line":"        if version \u003d\u003d 6:"},{"line_number":133,"context_line":"            iptables_manager.ipv6[\u0027raw\u0027].add_chain("},{"line_number":134,"context_line":"                DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":135,"context_line":"            iptables_manager.ipv6[\u0027raw\u0027].add_rule(\u0027PREROUTING\u0027, default_rule)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_c83760b0","line":132,"range":{"start_line":132,"start_character":22,"end_line":132,"end_character":23},"updated":"2019-04-24 15:43:10.000000000","message":"Please, use from neutron_lib.constants.IPv6","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_chain("},{"line_number":130,"context_line":"                DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":131,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_rule(\u0027PREROUTING\u0027, default_rule)"},{"line_number":132,"context_line":"        if version \u003d\u003d 6:"},{"line_number":133,"context_line":"            iptables_manager.ipv6[\u0027raw\u0027].add_chain("},{"line_number":134,"context_line":"                DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":135,"context_line":"            iptables_manager.ipv6[\u0027raw\u0027].add_rule(\u0027PREROUTING\u0027, default_rule)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_e83a6496","line":132,"range":{"start_line":132,"start_character":8,"end_line":132,"end_character":10},"updated":"2019-04-24 15:43:10.000000000","message":"elif","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_chain("},{"line_number":130,"context_line":"                DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":131,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_rule(\u0027PREROUTING\u0027, default_rule)"},{"line_number":132,"context_line":"        if version \u003d\u003d 6:"},{"line_number":133,"context_line":"            iptables_manager.ipv6[\u0027raw\u0027].add_chain("},{"line_number":134,"context_line":"                DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":135,"context_line":"            iptables_manager.ipv6[\u0027raw\u0027].add_rule(\u0027PREROUTING\u0027, default_rule)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_d102683d","line":132,"range":{"start_line":132,"start_character":22,"end_line":132,"end_character":23},"in_reply_to":"ffb9cba7_c83760b0","updated":"2019-04-29 11:28:39.000000000","message":"Done","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_chain("},{"line_number":130,"context_line":"                DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":131,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].add_rule(\u0027PREROUTING\u0027, default_rule)"},{"line_number":132,"context_line":"        if version \u003d\u003d 6:"},{"line_number":133,"context_line":"            iptables_manager.ipv6[\u0027raw\u0027].add_chain("},{"line_number":134,"context_line":"                DEFAULT_CONNTRACK_HELPER_CHAIN)"},{"line_number":135,"context_line":"            iptables_manager.ipv6[\u0027raw\u0027].add_rule(\u0027PREROUTING\u0027, default_rule)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_319744ef","line":132,"range":{"start_line":132,"start_character":8,"end_line":132,"end_character":10},"in_reply_to":"ffb9cba7_e83a6496","updated":"2019-04-29 11:28:39.000000000","message":"Done","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        iptables_manager.apply()"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _get_chain_rules_list(self, conntrack_helper, wrap_name):"},{"line_number":139,"context_line":"        chain_name \u003d (CONNTRACK_HELPER_CHAIN_PREFIX + conntrack_helper.id)["},{"line_number":140,"context_line":"                     :constants.MAX_IPTABLES_CHAIN_LEN_WRAP]"},{"line_number":141,"context_line":"        chain_rule_list \u003d []"},{"line_number":142,"context_line":"        chain_rule_list.append((DEFAULT_CONNTRACK_HELPER_CHAIN,"},{"line_number":143,"context_line":"                                \u0027-j %s-%s\u0027 % (wrap_name,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_e81d8473","line":140,"range":{"start_line":139,"start_character":8,"end_line":140,"end_character":60},"updated":"2019-04-24 15:43:10.000000000","message":"You use 3 times this assignation. Because it\u0027s a bit long, can you put this in a separate function? Something like \"_get_chain_name(id)\"","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    def _get_chain_rules_list(self, conntrack_helper, wrap_name):"},{"line_number":139,"context_line":"        chain_name \u003d (CONNTRACK_HELPER_CHAIN_PREFIX + conntrack_helper.id)["},{"line_number":140,"context_line":"                     :constants.MAX_IPTABLES_CHAIN_LEN_WRAP]"},{"line_number":141,"context_line":"        chain_rule_list \u003d []"},{"line_number":142,"context_line":"        chain_rule_list.append((DEFAULT_CONNTRACK_HELPER_CHAIN,"},{"line_number":143,"context_line":"                                \u0027-j %s-%s\u0027 % (wrap_name,"},{"line_number":144,"context_line":"                                              chain_name)))"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_c8dd2015","line":141,"updated":"2019-04-24 15:43:10.000000000","message":"nit: you can avoid this assigning L142 directly","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    def _get_chain_rules_list(self, conntrack_helper, wrap_name):"},{"line_number":139,"context_line":"        chain_name \u003d (CONNTRACK_HELPER_CHAIN_PREFIX + conntrack_helper.id)["},{"line_number":140,"context_line":"                     :constants.MAX_IPTABLES_CHAIN_LEN_WRAP]"},{"line_number":141,"context_line":"        chain_rule_list \u003d []"},{"line_number":142,"context_line":"        chain_rule_list.append((DEFAULT_CONNTRACK_HELPER_CHAIN,"},{"line_number":143,"context_line":"                                \u0027-j %s-%s\u0027 % (wrap_name,"},{"line_number":144,"context_line":"                                              chain_name)))"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_1d081190","line":141,"in_reply_to":"ffb9cba7_c8dd2015","updated":"2019-04-29 11:28:39.000000000","message":"Done","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        if (DEFAULT_CONNTRACK_HELPER_CHAIN not in"},{"line_number":173,"context_line":"                iptables_manager.ipv4[\u0027raw\u0027].chains):"},{"line_number":174,"context_line":"            self._install_default_rules(iptables_manager, 4)"},{"line_number":175,"context_line":"        if (DEFAULT_CONNTRACK_HELPER_CHAIN not in"},{"line_number":176,"context_line":"                iptables_manager.ipv6[\u0027raw\u0027].chains):"},{"line_number":177,"context_line":"            self._install_default_rules(iptables_manager, 6)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_08f1f8cd","line":174,"range":{"start_line":174,"start_character":58,"end_line":174,"end_character":59},"updated":"2019-04-24 15:43:10.000000000","message":"ditto, please use constants","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        if (DEFAULT_CONNTRACK_HELPER_CHAIN not in"},{"line_number":173,"context_line":"                iptables_manager.ipv4[\u0027raw\u0027].chains):"},{"line_number":174,"context_line":"            self._install_default_rules(iptables_manager, 4)"},{"line_number":175,"context_line":"        if (DEFAULT_CONNTRACK_HELPER_CHAIN not in"},{"line_number":176,"context_line":"                iptables_manager.ipv6[\u0027raw\u0027].chains):"},{"line_number":177,"context_line":"            self._install_default_rules(iptables_manager, 6)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_519c380e","line":174,"range":{"start_line":174,"start_character":58,"end_line":174,"end_character":59},"in_reply_to":"ffb9cba7_08f1f8cd","updated":"2019-04-29 11:28:39.000000000","message":"Done","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":174,"context_line":"            self._install_default_rules(iptables_manager, 4)"},{"line_number":175,"context_line":"        if (DEFAULT_CONNTRACK_HELPER_CHAIN not in"},{"line_number":176,"context_line":"                iptables_manager.ipv6[\u0027raw\u0027].chains):"},{"line_number":177,"context_line":"            self._install_default_rules(iptables_manager, 6)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        for conntrack_helper in conntrack_helpers:"},{"line_number":180,"context_line":"            self._rule_apply(iptables_manager, conntrack_helper)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_28ecfcf1","line":177,"range":{"start_line":177,"start_character":58,"end_line":177,"end_character":59},"updated":"2019-04-24 15:43:10.000000000","message":"ditto","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":174,"context_line":"            self._install_default_rules(iptables_manager, 4)"},{"line_number":175,"context_line":"        if (DEFAULT_CONNTRACK_HELPER_CHAIN not in"},{"line_number":176,"context_line":"                iptables_manager.ipv6[\u0027raw\u0027].chains):"},{"line_number":177,"context_line":"            self._install_default_rules(iptables_manager, 6)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        for conntrack_helper in conntrack_helpers:"},{"line_number":180,"context_line":"            self._rule_apply(iptables_manager, conntrack_helper)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_f19c4c0f","line":177,"range":{"start_line":177,"start_character":58,"end_line":177,"end_character":59},"in_reply_to":"ffb9cba7_28ecfcf1","updated":"2019-04-29 11:28:39.000000000","message":"Done","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                         :constants.MAX_IPTABLES_CHAIN_LEN_WRAP]"},{"line_number":215,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].remove_chain(chain_name)"},{"line_number":216,"context_line":"            iptables_manager.ipv6[\u0027raw\u0027].remove_chain(chain_name)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        iptables_manager.apply()"},{"line_number":219,"context_line":"        self.mapping.del_conntrack_helpers(conntrack_helpers)"},{"line_number":220,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_83286934","line":217,"updated":"2019-04-24 15:43:10.000000000","message":"Shouldn\u0027t you remove the DEFAULT_CONNTRACK_HELPER_CHAIN chain too?","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                         :constants.MAX_IPTABLES_CHAIN_LEN_WRAP]"},{"line_number":215,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].remove_chain(chain_name)"},{"line_number":216,"context_line":"            iptables_manager.ipv6[\u0027raw\u0027].remove_chain(chain_name)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        iptables_manager.apply()"},{"line_number":219,"context_line":"        self.mapping.del_conntrack_helpers(conntrack_helpers)"},{"line_number":220,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_91a63053","line":217,"in_reply_to":"ffb9cba7_83286934","updated":"2019-04-29 11:28:39.000000000","message":"We can have multiple conntrack helpers, the delete can be for one or more but not neccecarily all conntrack helpers. Each conntrack helper chain is nested under the DEFAULT chain, so we should delete the default chain.\n\nChain PREROUTING (policy ACCEPT)\ntarget     prot opt source               destination         \nneutron-l3-agent-PREROUTING  all  --  anywhere             anywhere            \n  \nChain neutron-l3-agent-PREROUTING (1 references)\ntarget     prot opt source               destination         \nneutron-l3-agent-cth  all  --  anywhere             anywhere            \n\nChain neutron-l3-agent-cth (1 references)\ntarget     prot opt source               destination         \nneutron-l3-agent-cth-da2a282  all  --  anywhere             anywhere            \n\nChain neutron-l3-agent-cth-da2a282 (1 references)\ntarget     prot opt source               destination         \nCT         udp  --  anywhere             anywhere             udp dpt:tftp CT helper tftp","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c9928f2e8a1fa45e3e5cc4b556c092916717dd11","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                         :constants.MAX_IPTABLES_CHAIN_LEN_WRAP]"},{"line_number":215,"context_line":"            iptables_manager.ipv4[\u0027raw\u0027].remove_chain(chain_name)"},{"line_number":216,"context_line":"            iptables_manager.ipv6[\u0027raw\u0027].remove_chain(chain_name)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        iptables_manager.apply()"},{"line_number":219,"context_line":"        self.mapping.del_conntrack_helpers(conntrack_helpers)"},{"line_number":220,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"dfbec78f_ac44cbb8","line":217,"in_reply_to":"ffb9cba7_91a63053","updated":"2019-05-02 16:25:10.000000000","message":"That\u0027s right!","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":225,"context_line":"        return router_info.iptables_manager"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def check_local_conntrack_helpers(self, context, router_info):"},{"line_number":228,"context_line":"        local_ct_helpers \u003d set(self.mapping.managed_conntrack_helpers.keys())"},{"line_number":229,"context_line":"        new_ct_helpers \u003d []"},{"line_number":230,"context_line":"        updated_cth_helpers \u003d []"},{"line_number":231,"context_line":"        current_ct_helpers \u003d set()"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_6390d54f","line":228,"range":{"start_line":228,"start_character":27,"end_line":228,"end_character":77},"updated":"2019-04-24 15:43:10.000000000","message":"IMO, you should have a function in ConntrackHelperMapping to retrieve this set, instead of accessing to this internal variable (which, BTW, I think should be private)","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":225,"context_line":"        return router_info.iptables_manager"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def check_local_conntrack_helpers(self, context, router_info):"},{"line_number":228,"context_line":"        local_ct_helpers \u003d set(self.mapping.managed_conntrack_helpers.keys())"},{"line_number":229,"context_line":"        new_ct_helpers \u003d []"},{"line_number":230,"context_line":"        updated_cth_helpers \u003d []"},{"line_number":231,"context_line":"        current_ct_helpers \u003d set()"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_ddf03900","line":228,"range":{"start_line":228,"start_character":27,"end_line":228,"end_character":77},"in_reply_to":"ffb9cba7_6390d54f","updated":"2019-04-29 11:28:39.000000000","message":"Done","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        updated_cth_helpers \u003d []"},{"line_number":231,"context_line":"        current_ct_helpers \u003d set()"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"        ct_helpers \u003d self.resource_rpc.bulk_pull("},{"line_number":234,"context_line":"            context, resources.CONNTRACKHELPER, filter_kwargs\u003d{"},{"line_number":235,"context_line":"                \u0027router_id\u0027: router_info.router[\u0027id\u0027]})"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_43a3519f","line":233,"range":{"start_line":233,"start_character":39,"end_line":233,"end_character":48},"updated":"2019-04-24 15:43:10.000000000","message":"You can just only pull this single resource, by using resource.pull:\n\nself.resource_rpc.pull(context, resources.CONNTRACKHELPER, router_info.router[\u0027id\u0027])","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c9928f2e8a1fa45e3e5cc4b556c092916717dd11","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        updated_cth_helpers \u003d []"},{"line_number":231,"context_line":"        current_ct_helpers \u003d set()"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"        ct_helpers \u003d self.resource_rpc.bulk_pull("},{"line_number":234,"context_line":"            context, resources.CONNTRACKHELPER, filter_kwargs\u003d{"},{"line_number":235,"context_line":"                \u0027router_id\u0027: router_info.router[\u0027id\u0027]})"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"dfbec78f_ec3ac332","line":233,"range":{"start_line":233,"start_character":39,"end_line":233,"end_character":48},"in_reply_to":"ffb9cba7_1d97b1ee","updated":"2019-05-02 16:25:10.000000000","message":"You are right!","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        updated_cth_helpers \u003d []"},{"line_number":231,"context_line":"        current_ct_helpers \u003d set()"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"        ct_helpers \u003d self.resource_rpc.bulk_pull("},{"line_number":234,"context_line":"            context, resources.CONNTRACKHELPER, filter_kwargs\u003d{"},{"line_number":235,"context_line":"                \u0027router_id\u0027: router_info.router[\u0027id\u0027]})"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_1d97b1ee","line":233,"range":{"start_line":233,"start_character":39,"end_line":233,"end_character":48},"in_reply_to":"ffb9cba7_43a3519f","updated":"2019-04-29 11:28:39.000000000","message":"Won\u0027t pull try to get a single CONNTRACKHELPER resource?\nWe want to get all the CONNTRACKHELPER resources associated with a router.","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a397afb7737d9944d44d24c79b12be260defcbcd","unresolved":false,"context_lines":[{"line_number":263,"context_line":"        self.check_local_conntrack_helpers(context, router_info)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    def add_router(self, context, data):"},{"line_number":266,"context_line":"        \"\"\"Handle a router add event."},{"line_number":267,"context_line":"        Called on router create."},{"line_number":268,"context_line":"        :param context: RPC context."},{"line_number":269,"context_line":"        :param data: Router data."}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_a881cc4c","line":266,"updated":"2019-04-24 15:43:10.000000000","message":"nit: the description of those functions is in the abs class. I don\u0027t think we need this here again.","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"b030f6d3e61d1a40a3aeeb4c03a3a6e014ef9a0d","unresolved":false,"context_lines":[{"line_number":263,"context_line":"        self.check_local_conntrack_helpers(context, router_info)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    def add_router(self, context, data):"},{"line_number":266,"context_line":"        \"\"\"Handle a router add event."},{"line_number":267,"context_line":"        Called on router create."},{"line_number":268,"context_line":"        :param context: RPC context."},{"line_number":269,"context_line":"        :param data: Router data."}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_d1d908f3","line":266,"in_reply_to":"ffb9cba7_a881cc4c","updated":"2019-04-29 11:28:39.000000000","message":"Done","commit_id":"bc81c559144f814a641cb0f109fa385d49732eee"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c9928f2e8a1fa45e3e5cc4b556c092916717dd11","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    def set_conntrack_helpers(self, conntrack_helpers):"},{"line_number":49,"context_line":"        for cth in conntrack_helpers:"},{"line_number":50,"context_line":"            self.__router_conntrack_helper_mapping[cth.router_id].add(cth.id)"},{"line_number":51,"context_line":"            self.__managed_conntrack_helpers[cth.id] \u003d cth"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def update_conntrack_helpers(self, conntrack_helpers):"},{"line_number":54,"context_line":"        for cth in conntrack_helpers:"}],"source_content_type":"text/x-python","patch_set":10,"id":"dfbec78f_ac728b10","line":51,"range":{"start_line":51,"start_character":17,"end_line":51,"end_character":19},"updated":"2019-05-02 16:25:10.000000000","message":"Hmmm I suggested to make them private (one single underscore), not to do name mangling (two underscores). IMO, this is not needed.","commit_id":"67109cf8ee112069612a325d710e5c5f466abe7a"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"8c3d0def62caf5a94c95fe93b1c2ea0cf66a8883","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    def set_conntrack_helpers(self, conntrack_helpers):"},{"line_number":49,"context_line":"        for cth in conntrack_helpers:"},{"line_number":50,"context_line":"            self.__router_conntrack_helper_mapping[cth.router_id].add(cth.id)"},{"line_number":51,"context_line":"            self.__managed_conntrack_helpers[cth.id] \u003d cth"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def update_conntrack_helpers(self, conntrack_helpers):"},{"line_number":54,"context_line":"        for cth in conntrack_helpers:"}],"source_content_type":"text/x-python","patch_set":10,"id":"dfbec78f_a4f6d7f8","line":51,"range":{"start_line":51,"start_character":17,"end_line":51,"end_character":19},"in_reply_to":"dfbec78f_ac728b10","updated":"2019-05-09 06:04:18.000000000","message":"Done","commit_id":"67109cf8ee112069612a325d710e5c5f466abe7a"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"29d274ad9b6bae738949cf83afe7dd89ba6756e0","unresolved":false,"context_lines":[{"line_number":265,"context_line":""},{"line_number":266,"context_line":"        self.check_local_conntrack_helpers(context, router_info)"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"    def add_router(self, context, data):"},{"line_number":269,"context_line":"        self.process_conntrack_helper(context, data)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def update_router(self, context, data):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dfbec78f_e24ecc6d","line":268,"range":{"start_line":268,"start_character":8,"end_line":268,"end_character":18},"updated":"2019-05-10 16:45:03.000000000","message":"If do not add synchronized here, the cache map may have race condition between router update its conntrack_helper and Line 112 conntrack_helper updating notification. For instance, router change the conntrack_helper to some one else, and the orignal conntrack_helper change the rule.\n\nBTW, we are now refactoring such lock extension lock granularity:\nhttps://review.opendev.org/#/c/656163/\nSuch bottleneck lock influence entire l3-agent router processing efficiency.\n\nBut it\u0027s OK to use the current style, we can refactor this later.","commit_id":"99802107ca7ba5790601fa9f1e8284372e564398"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"1e8edea490b554ce896a1d6d15d689f336e5001a","unresolved":false,"context_lines":[{"line_number":265,"context_line":""},{"line_number":266,"context_line":"        self.check_local_conntrack_helpers(context, router_info)"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"    def add_router(self, context, data):"},{"line_number":269,"context_line":"        self.process_conntrack_helper(context, data)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def update_router(self, context, data):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dfbec78f_247379b3","line":268,"range":{"start_line":268,"start_character":8,"end_line":268,"end_character":18},"in_reply_to":"dfbec78f_e24ecc6d","updated":"2019-05-13 11:39:11.000000000","message":"Thanks Yulong!\n\nDone.","commit_id":"99802107ca7ba5790601fa9f1e8284372e564398"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"29d274ad9b6bae738949cf83afe7dd89ba6756e0","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    def add_router(self, context, data):"},{"line_number":269,"context_line":"        self.process_conntrack_helper(context, data)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def update_router(self, context, data):"},{"line_number":272,"context_line":"        self.process_conntrack_helper(context, data)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"    def delete_router(self, context, data):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dfbec78f_023e808b","line":271,"range":{"start_line":271,"start_character":8,"end_line":271,"end_character":21},"updated":"2019-05-10 16:45:03.000000000","message":"ditto","commit_id":"99802107ca7ba5790601fa9f1e8284372e564398"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"1e8edea490b554ce896a1d6d15d689f336e5001a","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    def add_router(self, context, data):"},{"line_number":269,"context_line":"        self.process_conntrack_helper(context, data)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def update_router(self, context, data):"},{"line_number":272,"context_line":"        self.process_conntrack_helper(context, data)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"    def delete_router(self, context, data):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dfbec78f_c46b7db4","line":271,"range":{"start_line":271,"start_character":8,"end_line":271,"end_character":21},"in_reply_to":"dfbec78f_023e808b","updated":"2019-05-13 11:39:11.000000000","message":"Done","commit_id":"99802107ca7ba5790601fa9f1e8284372e564398"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"5762b9d7a7588d0f2d1371baa76b495a91cfa1fa","unresolved":false,"context_lines":[{"line_number":265,"context_line":""},{"line_number":266,"context_line":"        self.check_local_conntrack_helpers(context, router_info)"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"    @lockutils.synchronized(\u0027conntrack-helpers\u0027)"},{"line_number":269,"context_line":"    def add_router(self, context, data):"},{"line_number":270,"context_line":"        self.process_conntrack_helper(context, data)"},{"line_number":271,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"7faddb67_26feab6b","line":268,"range":{"start_line":268,"start_character":4,"end_line":268,"end_character":48},"updated":"2019-07-22 14:26:23.000000000","message":"Is it possible to refactor such centralized lock?\nWe already have done some minimizing work.\nPlz see this bug for more information:\nhttps://bugs.launchpad.net/neutron/+bug/1824911","commit_id":"08efe64b122e2e32f44ad6950f37fac89a0610be"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"5250c971175725a53e76db7b6183473b36e941f3","unresolved":false,"context_lines":[{"line_number":265,"context_line":""},{"line_number":266,"context_line":"        self.check_local_conntrack_helpers(context, router_info)"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"    @lockutils.synchronized(\u0027conntrack-helpers\u0027)"},{"line_number":269,"context_line":"    def add_router(self, context, data):"},{"line_number":270,"context_line":"        self.process_conntrack_helper(context, data)"},{"line_number":271,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"7faddb67_0fea3b0d","line":268,"range":{"start_line":268,"start_character":4,"end_line":268,"end_character":48},"in_reply_to":"7faddb67_26feab6b","updated":"2019-08-29 10:57:40.000000000","message":"I have the same opinion, but \"process_conntrack_helper\" and then \"check_local_conntrack_helpers\" must be sync entirely.","commit_id":"08efe64b122e2e32f44ad6950f37fac89a0610be"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"5762b9d7a7588d0f2d1371baa76b495a91cfa1fa","unresolved":false,"context_lines":[{"line_number":269,"context_line":"    def add_router(self, context, data):"},{"line_number":270,"context_line":"        self.process_conntrack_helper(context, data)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"    @lockutils.synchronized(\u0027conntrack-helpers\u0027)"},{"line_number":273,"context_line":"    def update_router(self, context, data):"},{"line_number":274,"context_line":"        self.process_conntrack_helper(context, data)"},{"line_number":275,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"7faddb67_a6461bc0","line":272,"range":{"start_line":272,"start_character":4,"end_line":272,"end_character":48},"updated":"2019-07-22 14:26:23.000000000","message":"ditto","commit_id":"08efe64b122e2e32f44ad6950f37fac89a0610be"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b9fe5ad28a88683c23d98e06cdd168e1b77a5e02","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            router_info \u003d self.agent_api.get_router_info("},{"line_number":116,"context_line":"                conntrack_helper.router_id)"},{"line_number":117,"context_line":"            if not router_info:"},{"line_number":118,"context_line":"                return"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"            iptables_manager \u003d self._get_iptables_manager(router_info)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"7faddb67_7df8387b","line":118,"updated":"2019-08-30 08:53:16.000000000","message":"Maybe You should add here same debug message like in L262?","commit_id":"b8576b7be207c1e11aa243c4aa7ba1601f6e12bc"}],"neutron/tests/functional/agent/l3/extensions/test_conntrack_helper_extension.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"7209dbeb84c2258d2602480e335ec95e557239c4","unresolved":false,"context_lines":[{"line_number":100,"context_line":"            return (rule_obj in exising_ipv4_rules and"},{"line_number":101,"context_line":"                    rule_obj in exising_ipv6_rules)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"            common_utils.wait_until_true(check_chain_rules_set)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def _test_centralized_routers(self, router_info):"},{"line_number":106,"context_line":"        router_id \u003d router_info[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_dddfec37","line":103,"range":{"start_line":103,"start_character":12,"end_line":103,"end_character":63},"updated":"2019-07-12 09:32:06.000000000","message":"is this indentation correct? IMO it should be outside of check_chain_rules_set() function because now it\u0027s inside of this function so this wait_until_true is never called","commit_id":"bfb29d0bbdd35df91e69f80643e6bef87a7f5bd8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"395eda7c124522af53842e028b74471d453c876e","unresolved":false,"context_lines":[{"line_number":100,"context_line":"            return (rule_obj in exising_ipv4_rules and"},{"line_number":101,"context_line":"                    rule_obj in exising_ipv6_rules)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"            common_utils.wait_until_true(check_chain_rules_set)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def _test_centralized_routers(self, router_info):"},{"line_number":106,"context_line":"        router_id \u003d router_info[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_1d756bcb","line":103,"range":{"start_line":103,"start_character":12,"end_line":103,"end_character":63},"in_reply_to":"7faddb67_dddfec37","updated":"2019-07-15 10:32:50.000000000","message":"Done","commit_id":"bfb29d0bbdd35df91e69f80643e6bef87a7f5bd8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"a8c223845de0082c63fb12ba5e367da6eeb60714","unresolved":false,"context_lines":[{"line_number":100,"context_line":"            return (rule_obj in exising_ipv4_rules and"},{"line_number":101,"context_line":"                    rule_obj in exising_ipv6_rules)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        common_utils.wait_until_true(check_chain_rules_set)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def _test_centralized_routers(self, router_info):"},{"line_number":106,"context_line":"        router_id \u003d router_info[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":21,"id":"7faddb67_c2c83823","line":103,"range":{"start_line":103,"start_character":37,"end_line":103,"end_character":58},"updated":"2019-07-16 12:03:38.000000000","message":"After correcting the intentation error here, it turns out check_chain_rules_set never returned true.","commit_id":"99b99589fdbb860bf266650584c3588065e12afe"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"5250c971175725a53e76db7b6183473b36e941f3","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        ri \u003d self.manage_router(self.agent, router_info)"},{"line_number":112,"context_line":"        for cthobj in self.conntrack_helpers:"},{"line_number":113,"context_line":"            self._assert_conntrack_helper_iptables_is_set(ri, cthobj)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"class TestL3AgentConntrackHelperExtension("},{"line_number":117,"context_line":"        test_dvr_router.DvrRouterTestFramework,"}],"source_content_type":"text/x-python","patch_set":24,"id":"7faddb67_6f3e4fba","line":114,"updated":"2019-08-29 10:57:40.000000000","message":"Maybe you should also check:\n- When a router is updated\n- When a router is deleted","commit_id":"b8576b7be207c1e11aa243c4aa7ba1601f6e12bc"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b9fe5ad28a88683c23d98e06cdd168e1b77a5e02","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        ri \u003d self.manage_router(self.agent, router_info)"},{"line_number":112,"context_line":"        for cthobj in self.conntrack_helpers:"},{"line_number":113,"context_line":"            self._assert_conntrack_helper_iptables_is_set(ri, cthobj)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"class TestL3AgentConntrackHelperExtension("},{"line_number":117,"context_line":"        test_dvr_router.DvrRouterTestFramework,"}],"source_content_type":"text/x-python","patch_set":24,"id":"7faddb67_1d1824d1","line":114,"in_reply_to":"7faddb67_6f3e4fba","updated":"2019-08-30 08:53:16.000000000","message":"+1.\nAnd e.g. when cth is removed to check if it is really cleared from iptables.","commit_id":"b8576b7be207c1e11aa243c4aa7ba1601f6e12bc"}],"setup.cfg":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6daf70e16c2f387e4e5f8354a7fe09e0a7b7efe1","unresolved":false,"context_lines":[{"line_number":117,"context_line":"    gateway_ip_qos \u003d neutron.agent.l3.extensions.qos.gateway_ip:RouterGatewayIPQosAgentExtension"},{"line_number":118,"context_line":"    port_forwarding \u003d neutron.agent.l3.extensions.port_forwarding:PortForwardingAgentExtension"},{"line_number":119,"context_line":"    snat_log \u003d neutron.agent.l3.extensions.snat_log:SNATLoggingExtension"},{"line_number":120,"context_line":"    conntrack_helper \u003d neutron.agent.l3.extensions.conntrack_helper:ConntrackHelperAgentExtension"},{"line_number":121,"context_line":"neutron.services.logapi.drivers \u003d"},{"line_number":122,"context_line":"    ovs \u003d neutron.services.logapi.drivers.openvswitch.ovs_firewall_log:OVSFirewallLoggingDriver"},{"line_number":123,"context_line":"neutron.qos.agent_drivers \u003d"}],"source_content_type":"text/x-ttcn-cfg","patch_set":13,"id":"bfb3d3c7_44aa86d2","line":120,"range":{"start_line":120,"start_character":4,"end_line":120,"end_character":20},"updated":"2019-05-21 15:54:07.000000000","message":"If you will add some tempest cases, you may need to enable this to the neutron test devstack plugin, here are some example:\nhttps://github.com/openstack/neutron/blob/master/devstack/plugin.sh#L77-L82","commit_id":"f74e29758a63f4f7350c79d451274cba8d77e1ad"}]}