)]}'
{"neutron/agent/l3/dvr_edge_router.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"74f101d41657f38c587c8f08ccaa4a59655e45ff","unresolved":false,"context_lines":[{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    def perform_snat_action(self, snat_callback, *args):"},{"line_number":167,"context_line":"        # NOTE DVR skips this step in a few cases..."},{"line_number":168,"context_line":"        if not self.get_ex_gw_port():"},{"line_number":169,"context_line":"            return"},{"line_number":170,"context_line":"        if not self._is_this_snat_host():"},{"line_number":171,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a41bdd9_5d55f3c3","side":"PARENT","line":168,"updated":"2015-07-17 15:47:29.000000000","message":"These checks are just as good under _handle_router_snat_rules.  See L153 - L156.","commit_id":"0d93458d1ed1d16fdac6b9f63b4cca8b13bcb619"},{"author":{"_account_id":105,"name":"Kyle Mestery","email":"mestery@mestery.com","username":"mestery"},"change_message_id":"51a2febf1c310f11147a4df3b98c7f875dcb385b","unresolved":false,"context_lines":[{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    def perform_snat_action(self, snat_callback, *args):"},{"line_number":167,"context_line":"        # NOTE DVR skips this step in a few cases..."},{"line_number":168,"context_line":"        if not self.get_ex_gw_port():"},{"line_number":169,"context_line":"            return"},{"line_number":170,"context_line":"        if not self._is_this_snat_host():"},{"line_number":171,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a41bdd9_bd9e7f52","side":"PARENT","line":168,"in_reply_to":"9a41bdd9_5d55f3c3","updated":"2015-07-17 15:52:40.000000000","message":"Seems reasonable.","commit_id":"0d93458d1ed1d16fdac6b9f63b4cca8b13bcb619"}],"neutron/agent/l3/router_info.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e04a5b18384aa40a48458e7268e0e9e579076d30","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        # enable_snat by default if it wasn\u0027t specified by plugin"},{"line_number":99,"context_line":"        self._snat_enabled \u003d self._router.get(\u0027enable_snat\u0027, True)"},{"line_number":100,"context_line":"        # Set a SNAT action for the router"},{"line_number":101,"context_line":"        if self._router.get(\u0027gw_port\u0027):"},{"line_number":102,"context_line":"            self._snat_action \u003d (\u0027add_rules\u0027 if self._snat_enabled"},{"line_number":103,"context_line":"                                 else \u0027remove_rules\u0027)"},{"line_number":104,"context_line":"        elif self.ex_gw_port:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba3cc151_239134f2","side":"PARENT","line":101,"updated":"2015-07-10 00:03:42.000000000","message":"remove_rules doesn\u0027t do anything and the check for gw_port was already on L566.  So, I think this is completely unnecessary to set \u0027add_rules\u0027 or \u0027remove_rules\u0027.  It is just confusing.","commit_id":"d22110a1d189985ab836f301ab415c4d0511de4e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e04a5b18384aa40a48458e7268e0e9e579076d30","unresolved":false,"context_lines":[{"line_number":125,"context_line":"            snat_callback(self._router.get(\u0027gw_port\u0027),"},{"line_number":126,"context_line":"                          *args,"},{"line_number":127,"context_line":"                          action\u003dself._snat_action)"},{"line_number":128,"context_line":"        self._snat_action \u003d None"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def _update_routing_table(self, operation, route):"},{"line_number":131,"context_line":"        cmd \u003d [\u0027ip\u0027, \u0027route\u0027, operation, \u0027to\u0027, route[\u0027destination\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba3cc151_437c78b5","side":"PARENT","line":128,"updated":"2015-07-10 00:03:42.000000000","message":"self._snat_action gets reset each time the router is processed.  _handle_router_snat_rules was only getting called once anyway.  I don\u0027t know why we need to set this to None.","commit_id":"d22110a1d189985ab836f301ab415c4d0511de4e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e04a5b18384aa40a48458e7268e0e9e579076d30","unresolved":false,"context_lines":[{"line_number":547,"context_line":""},{"line_number":548,"context_line":"    def _add_snat_rules(self, ex_gw_port, iptables_manager,"},{"line_number":549,"context_line":"                        interface_name):"},{"line_number":550,"context_line":"        if self._snat_enabled and ex_gw_port:"},{"line_number":551,"context_line":"            # ex_gw_port should not be None in this case"},{"line_number":552,"context_line":"            # NAT rules are added only if ex_gw_port has an IPv4 address"},{"line_number":553,"context_line":"            for ip_addr in ex_gw_port[\u0027fixed_ips\u0027]:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba3cc151_2356142b","line":550,"updated":"2015-07-10 00:03:42.000000000","message":"As far as I can tell.  This boils down to \"is snat enabled\".  So, why not just say that?","commit_id":"af810b7091982d13fb780452883fcb3929fc4be5"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"74f101d41657f38c587c8f08ccaa4a59655e45ff","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        self._snat_enabled \u003d self._router.get(\u0027enable_snat\u0027, True)"},{"line_number":100,"context_line":"        # Set a SNAT action for the router"},{"line_number":101,"context_line":"        if self._router.get(\u0027gw_port\u0027):"},{"line_number":102,"context_line":"            self._snat_action \u003d (\u0027add_rules\u0027 if self._snat_enabled"},{"line_number":103,"context_line":"                                 else \u0027remove_rules\u0027)"},{"line_number":104,"context_line":"        elif self.ex_gw_port:"},{"line_number":105,"context_line":"            # Gateway port was removed, remove rules"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a41bdd9_3decaf28","side":"PARENT","line":102,"updated":"2015-07-17 15:47:29.000000000","message":"The only actions are \u0027add_rules\u0027 and \u0027remove_rules\u0027.  The latter doesn\u0027t do anything and the former is set when self.ex_gw_port is True and self._snat_enabled is True.  Both conditions are now checked directly on L550 of the new code.","commit_id":"0d93458d1ed1d16fdac6b9f63b4cca8b13bcb619"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"74f101d41657f38c587c8f08ccaa4a59655e45ff","unresolved":false,"context_lines":[{"line_number":125,"context_line":"            snat_callback(self._router.get(\u0027gw_port\u0027),"},{"line_number":126,"context_line":"                          *args,"},{"line_number":127,"context_line":"                          action\u003dself._snat_action)"},{"line_number":128,"context_line":"        self._snat_action \u003d None"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def _update_routing_table(self, operation, route):"},{"line_number":131,"context_line":"        cmd \u003d [\u0027ip\u0027, \u0027route\u0027, operation, \u0027to\u0027, route[\u0027destination\u0027],"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a41bdd9_7d2f9788","side":"PARENT","line":128,"updated":"2015-07-17 15:47:29.000000000","message":"Setting this to None here seemed to be to ensure that the action was only run once.  It would get reset on L102 each time we processed the router and perform_snat_action is only called once (L537) each time a router is processed.  So, I can\u0027t see any purpose for this.","commit_id":"0d93458d1ed1d16fdac6b9f63b4cca8b13bcb619"}]}