)]}'
{"neutron/agent/l3/dvr_edge_router.py":[{"author":{"_account_id":25618,"name":"Paweł Suder","email":"pawel@suder.info","username":"SuderPawel"},"change_message_id":"9995be3e44dbb3b9dfb7eb0c5cc401b8b08b8cce","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        with self.snat_iptables_manager.defer_apply():"},{"line_number":196,"context_line":"            self._empty_snat_chains(self.snat_iptables_manager)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            floatingips \u003d self.get_floating_ips()"},{"line_number":199,"context_line":"            if floatingips:"},{"line_number":200,"context_line":"                self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule(\u0027snat\u0027,"},{"line_number":201,"context_line":"                                                            \u0027-j $float-snat\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"df87a7cf_970cae94","line":198,"range":{"start_line":198,"start_character":26,"end_line":198,"end_character":49},"updated":"2017-12-11 08:00:29.000000000","message":"Shouldn\u0027t you mock self.get_floating_ips() or have some scenarios for different results of this method?","commit_id":"e3889a62cea3cc87835daf5f52a4dcc2bbbd15af"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"878c091421d8f18d3cbe5e81f0f16cce28868483","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        with self.snat_iptables_manager.defer_apply():"},{"line_number":196,"context_line":"            self._empty_snat_chains(self.snat_iptables_manager)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            floatingips \u003d self.get_floating_ips()"},{"line_number":199,"context_line":"            if floatingips:"},{"line_number":200,"context_line":"                self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule(\u0027snat\u0027,"},{"line_number":201,"context_line":"                                                            \u0027-j $float-snat\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"df87a7cf_59716775","line":198,"range":{"start_line":198,"start_character":26,"end_line":198,"end_character":49},"in_reply_to":"df87a7cf_970cae94","updated":"2017-12-11 20:45:24.000000000","message":"Basicaly test cases need to be added to this change","commit_id":"e3889a62cea3cc87835daf5f52a4dcc2bbbd15af"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"878c091421d8f18d3cbe5e81f0f16cce28868483","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            self._empty_snat_chains(self.snat_iptables_manager)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            floatingips \u003d self.get_floating_ips()"},{"line_number":199,"context_line":"            if floatingips:"},{"line_number":200,"context_line":"                self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule(\u0027snat\u0027,"},{"line_number":201,"context_line":"                                                            \u0027-j $float-snat\u0027)"},{"line_number":202,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"df87a7cf_39e903e5","line":199,"range":{"start_line":199,"start_character":15,"end_line":199,"end_character":26},"updated":"2017-12-11 20:45:24.000000000","message":"why do you need to define the floatingips variable.\nYou can simply call the if statement on the self.get_floating_ips()","commit_id":"e3889a62cea3cc87835daf5f52a4dcc2bbbd15af"},{"author":{"_account_id":25618,"name":"Paweł Suder","email":"pawel@suder.info","username":"SuderPawel"},"change_message_id":"9995be3e44dbb3b9dfb7eb0c5cc401b8b08b8cce","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            floatingips \u003d self.get_floating_ips()"},{"line_number":199,"context_line":"            if floatingips:"},{"line_number":200,"context_line":"                self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule(\u0027snat\u0027,"},{"line_number":201,"context_line":"                                                            \u0027-j $float-snat\u0027)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"            # NOTE: DVR adds the jump to float snat via super class,"},{"line_number":204,"context_line":"            # but that is in the router namespace and not snat."}],"source_content_type":"text/x-python","patch_set":1,"id":"df87a7cf_371b62ca","line":201,"range":{"start_line":201,"start_character":60,"end_line":201,"end_character":76},"updated":"2017-12-11 08:00:29.000000000","message":"Pep8 should say that it has incorrect indentation.","commit_id":"e3889a62cea3cc87835daf5f52a4dcc2bbbd15af"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"878c091421d8f18d3cbe5e81f0f16cce28868483","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            floatingips \u003d self.get_floating_ips()"},{"line_number":199,"context_line":"            if floatingips:"},{"line_number":200,"context_line":"                self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule(\u0027snat\u0027,"},{"line_number":201,"context_line":"                                                            \u0027-j $float-snat\u0027)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"            # NOTE: DVR adds the jump to float snat via super class,"},{"line_number":204,"context_line":"            # but that is in the router namespace and not snat."}],"source_content_type":"text/x-python","patch_set":1,"id":"df87a7cf_3976e371","line":201,"range":{"start_line":201,"start_character":60,"end_line":201,"end_character":76},"in_reply_to":"df87a7cf_371b62ca","updated":"2017-12-11 20:45:24.000000000","message":"+1","commit_id":"e3889a62cea3cc87835daf5f52a4dcc2bbbd15af"},{"author":{"_account_id":25618,"name":"Paweł Suder","email":"pawel@suder.info","username":"SuderPawel"},"change_message_id":"48c0f98cd4c347455489ed6b43e660c9321aef2b","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            for fip in self.get_floating_ips():"},{"line_number":199,"context_line":"                if fip.get(n_const.DVR_SNAT_BOUND):"},{"line_number":200,"context_line":"                    self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule(\u0027snat\u0027,"},{"line_number":201,"context_line":"                                                            \u0027-j $float-snat\u0027)"},{"line_number":202,"context_line":"                    break"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"            # NOTE: DVR adds the jump to float snat via super class,"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_b9574526","line":201,"range":{"start_line":201,"start_character":60,"end_line":201,"end_character":77},"updated":"2017-12-12 09:55:12.000000000","message":"Still I have some bad feelings about it. It should look like this\n\n  self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule(\u0027snat\u0027,\n                                                  \u0027-j $float-snat\u0027)\n\nOr\n\n  self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule(\n      \u0027snat\u0027, \u0027-j $float-snat\u0027)","commit_id":"5bd4a81a81268c65a7c212be7a87a5be64262893"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"85165892503332c0122883869c06ae50de342f39","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            self._empty_snat_chains(self.snat_iptables_manager)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            for fip in self.get_floating_ips():"},{"line_number":199,"context_line":"                if fip.get(n_const.DVR_SNAT_BOUND):"},{"line_number":200,"context_line":"                    self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule("},{"line_number":201,"context_line":"                        \u0027snat\u0027, \u0027-j $float-snat\u0027)"},{"line_number":202,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f91af0f_ab4f8290","line":199,"updated":"2018-01-04 16:36:40.000000000","message":"I\u0027m not sure we need this check, since it seems Ok to install this rule whether we have a centralized FIP or not, right?  Then it could just live below the comment.","commit_id":"196a1a8325f1364963f58c7a5d9fb8b01b0bef55"},{"author":{"_account_id":23430,"name":"sunzuohua","email":"zuohuasun@gmail.com","username":"ZuohuaSun"},"change_message_id":"a89a3f07f08cece36267923329c708a02d682866","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            self._empty_snat_chains(self.snat_iptables_manager)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            for fip in self.get_floating_ips():"},{"line_number":199,"context_line":"                if fip.get(n_const.DVR_SNAT_BOUND):"},{"line_number":200,"context_line":"                    self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule("},{"line_number":201,"context_line":"                        \u0027snat\u0027, \u0027-j $float-snat\u0027)"},{"line_number":202,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f91af0f_8e0cde41","line":199,"in_reply_to":"9f91af0f_ab4f8290","updated":"2018-01-08 01:11:45.000000000","message":"Yes, you are right.The check can be removed.","commit_id":"196a1a8325f1364963f58c7a5d9fb8b01b0bef55"},{"author":{"_account_id":21781,"name":"Hunt Xu","email":"mhuntxu@gmail.com","username":"huntxu"},"change_message_id":"abe0d5ba30892af15f6c5b94371fc0fa12d3729e","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        with self.snat_iptables_manager.defer_apply():"},{"line_number":196,"context_line":"            self._empty_snat_chains(self.snat_iptables_manager)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            for fip in self.get_floating_ips():"},{"line_number":199,"context_line":"                self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule("},{"line_number":200,"context_line":"                    \u0027snat\u0027, \u0027-j $float-snat\u0027)"},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9f91af0f_51451742","line":198,"updated":"2018-01-08 01:27:51.000000000","message":"This loop makes add_rule below run multiple times, even though once is enough.\n\nCan we change this to something like \"if self.get_floating_ips():\"?","commit_id":"5ec09babdca5065475b8e19271e54b1c69b92bbb"},{"author":{"_account_id":23430,"name":"sunzuohua","email":"zuohuasun@gmail.com","username":"ZuohuaSun"},"change_message_id":"dad59b1ce5d19d8e81c830d44662b6ccfd5ae723","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        with self.snat_iptables_manager.defer_apply():"},{"line_number":196,"context_line":"            self._empty_snat_chains(self.snat_iptables_manager)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            for fip in self.get_floating_ips():"},{"line_number":199,"context_line":"                self.snat_iptables_manager.ipv4[\u0027nat\u0027].add_rule("},{"line_number":200,"context_line":"                    \u0027snat\u0027, \u0027-j $float-snat\u0027)"},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9f91af0f_a2a1b396","line":198,"in_reply_to":"9f91af0f_51451742","updated":"2018-01-08 06:43:42.000000000","message":"Sorry, I make a mistake.","commit_id":"5ec09babdca5065475b8e19271e54b1c69b92bbb"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"dcd36abbbcc64c99dbfa37256fd0e041135ef74d","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                \u0027snat\u0027, \u0027-j $float-snat\u0027)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"            # NOTE: DVR adds the jump to float snat via super class,"},{"line_number":202,"context_line":"            # but that is in the router namespace and not snat."},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"            self._add_snat_rules(ex_gw_port, self.snat_iptables_manager,"},{"line_number":205,"context_line":"                                 interface_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f91af0f_c3b38f97","line":202,"updated":"2018-01-08 16:15:17.000000000","message":"nit: can you just move this comment, as it references the line you just added.","commit_id":"0c02188926197c21957215a73475bf0b4a831288"}],"neutron/tests/unit/agent/l3/test_dvr_local_router.py":[{"author":{"_account_id":25618,"name":"Paweł Suder","email":"pawel@suder.info","username":"SuderPawel"},"change_message_id":"48c0f98cd4c347455489ed6b43e660c9321aef2b","unresolved":false,"context_lines":[{"line_number":779,"context_line":"            self.assertFalse(ri.get_router_cidrs(device))"},{"line_number":780,"context_line":""},{"line_number":781,"context_line":"    @mock.patch.object(dvr_router.DvrLocalRouter, \u0027_handle_router_snat_rules\u0027)"},{"line_number":782,"context_line":"    def test_handle_fip_snat_rule_for_centralized_fip(self,"},{"line_number":783,"context_line":"                                                _handle_router_snat_rules):"},{"line_number":784,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"},{"line_number":785,"context_line":"        agent.conf.agent_mode \u003d lib_constants.L3_AGENT_MODE_DVR_SNAT"},{"line_number":786,"context_line":"        self.mock_driver.unplug.reset_mock()"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_190bd129","line":783,"range":{"start_line":782,"start_character":59,"end_line":783,"end_character":48},"updated":"2017-12-12 09:55:12.000000000","message":"That indentation also should be noticed by pep8 as incorrect.","commit_id":"5bd4a81a81268c65a7c212be7a87a5be64262893"},{"author":{"_account_id":25618,"name":"Paweł Suder","email":"pawel@suder.info","username":"SuderPawel"},"change_message_id":"5d0481b52c18de7b9ea4120254916b01e848213f","unresolved":false,"context_lines":[{"line_number":779,"context_line":"            self.assertFalse(ri.get_router_cidrs(device))"},{"line_number":780,"context_line":""},{"line_number":781,"context_line":"    @mock.patch.object(dvr_router.DvrLocalRouter, \u0027_handle_router_snat_rules\u0027)"},{"line_number":782,"context_line":"    def test_handle_snat_rule_for_centralized_fip(self, _handle_router_snat_rules):"},{"line_number":783,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"},{"line_number":784,"context_line":"        agent.conf.agent_mode \u003d lib_constants.L3_AGENT_MODE_DVR_SNAT"},{"line_number":785,"context_line":"        self.mock_driver.unplug.reset_mock()"}],"source_content_type":"text/x-python","patch_set":4,"id":"df87a7cf_2da9ab8e","line":782,"range":{"start_line":782,"start_character":4,"end_line":782,"end_character":83},"updated":"2017-12-12 12:40:49.000000000","message":"Too long..\n\n  \u003e\u003e\u003e len(\"    def test_handle_snat_rule_for_centralized_fip(self, _handle_router_snat_rules):\")\n  83","commit_id":"86a8ffbd1143e7d3fcdfe993933edd22d856b93b"}]}