)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"b50adf2678d2e69cce903eccf6904140ea86c67b","unresolved":false,"context_lines":[{"line_number":13,"context_line":"access resources which are not created."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"In this change, agent will have new router_info(i.e"},{"line_number":16,"context_line":"self.router_info[router_id] \u003d ri) only when initialize() success."},{"line_number":17,"context_line":"When initialize() fails, as router_info is not part of agent,"},{"line_number":18,"context_line":"\"_process_router_if_compatible\" will again call initialize()."},{"line_number":19,"context_line":"We also cleanup router_info when initialize() fails."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9a30ddce_f4f03ccb","line":16,"range":{"start_line":16,"start_character":57,"end_line":16,"end_character":64},"updated":"2017-03-11 00:40:00.000000000","message":"succeeds","commit_id":"c6ebd7c50241dae2eff880d0f6fcafc437b22145"}],"neutron/agent/l3/agent.py":[{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"ad2eda63f85dca9c541efed6414c85f7b4bd6fa1","unresolved":false,"context_lines":[{"line_number":347,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":348,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":349,"context_line":"                              router_id)"},{"line_number":350,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":351,"context_line":"                ri.delete()"},{"line_number":352,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":353,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a430d35_7a7ef36a","line":350,"range":{"start_line":350,"start_character":16,"end_line":350,"end_character":72},"updated":"2017-02-08 17:48:06.000000000","message":"I do think you might also need to clean the snat namespace if it was created and if it is a dvr router.\nns_manager.ensure_snat_cleanup(router_id)\n\nLet me know your thoughts.","commit_id":"5c7550197cdc261656f743f40533c91a455d0610"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"eeb671e9c57b608af620a1217c9a911eeb098ace","unresolved":false,"context_lines":[{"line_number":347,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":348,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":349,"context_line":"                              router_id)"},{"line_number":350,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":351,"context_line":"                ri.delete()"},{"line_number":352,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":353,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a430d35_6d9e0c65","line":350,"range":{"start_line":350,"start_character":16,"end_line":350,"end_character":72},"in_reply_to":"1a430d35_4875d0c8","updated":"2017-02-09 17:59:17.000000000","message":"Yes I see it. Thanks.","commit_id":"5c7550197cdc261656f743f40533c91a455d0610"},{"author":{"_account_id":17821,"name":"Zhixin Li","email":"lizhixin@awcloud.com","username":"lizhixin"},"change_message_id":"37306aa6a15357c9b6cd783575ea04676d4a7594","unresolved":false,"context_lines":[{"line_number":347,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":348,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":349,"context_line":"                              router_id)"},{"line_number":350,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":351,"context_line":"                ri.delete()"},{"line_number":352,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":353,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a430d35_4875d0c8","line":350,"range":{"start_line":350,"start_character":16,"end_line":350,"end_character":72},"in_reply_to":"1a430d35_7a7ef36a","updated":"2017-02-09 12:34:46.000000000","message":"I had a test and found that the ensure_router_cleanup method will clean both snat and qrouter namespace.","commit_id":"5c7550197cdc261656f743f40533c91a455d0610"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"cf031004c7071a19694b3828d9e973c920e042ca","unresolved":false,"context_lines":[{"line_number":347,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":348,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":349,"context_line":"                              router_id)"},{"line_number":350,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":351,"context_line":"                ri.delete()"},{"line_number":352,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":353,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a430d35_3598dd77","line":350,"range":{"start_line":350,"start_character":16,"end_line":350,"end_character":72},"in_reply_to":"1a430d35_7a7ef36a","updated":"2017-02-09 12:00:57.000000000","message":"Yes. You are correct. I thought below ri.delete will call [1]\nwhich deletes snat namespace. Am I correct?\n\n[1] https://github.com/openstack/neutron/blob/master/neutron/agent/l3/dvr_edge_router.py#L217","commit_id":"5c7550197cdc261656f743f40533c91a455d0610"},{"author":{"_account_id":17821,"name":"Zhixin Li","email":"lizhixin@awcloud.com","username":"lizhixin"},"change_message_id":"37306aa6a15357c9b6cd783575ea04676d4a7594","unresolved":false,"context_lines":[{"line_number":348,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":349,"context_line":"                              router_id)"},{"line_number":350,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":351,"context_line":"                ri.delete()"},{"line_number":352,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def _safe_router_removed(self, router_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a430d35_cb2b7a80","line":351,"range":{"start_line":351,"start_character":16,"end_line":351,"end_character":27},"updated":"2017-02-09 12:34:46.000000000","message":"Here will execute into get_ha_device_name method in ha_router.py, where it will fetch the ha_port and raise an error.","commit_id":"5c7550197cdc261656f743f40533c91a455d0610"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"944bdcd9e5149df62e9a111224ee7d189ac17011","unresolved":false,"context_lines":[{"line_number":348,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":349,"context_line":"                              router_id)"},{"line_number":350,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":351,"context_line":"                ri.delete()"},{"line_number":352,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def _safe_router_removed(self, router_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a430d35_abf1c96c","line":351,"range":{"start_line":351,"start_character":16,"end_line":351,"end_character":27},"in_reply_to":"1a430d35_41d92bf2","updated":"2017-02-09 14:32:56.000000000","message":"If delete raise error, it is handled in _process_router_update(which again calls _process_added_router). Added new test [1] for that.\n\n[1] https://review.openstack.org/#/c/431026/3/neutron/tests/unit/agent/l3/test_agent.py@1726","commit_id":"5c7550197cdc261656f743f40533c91a455d0610"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"391b52158a7bc116ab37dd40d4fe67e92f01f63b","unresolved":false,"context_lines":[{"line_number":348,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":349,"context_line":"                              router_id)"},{"line_number":350,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":351,"context_line":"                ri.delete()"},{"line_number":352,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def _safe_router_removed(self, router_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a430d35_41d92bf2","line":351,"range":{"start_line":351,"start_character":16,"end_line":351,"end_character":27},"in_reply_to":"1a430d35_cb2b7a80","updated":"2017-02-09 13:38:58.000000000","message":"I will test, what will be the behavior if ri.delete() raises any exception. Thanks","commit_id":"5c7550197cdc261656f743f40533c91a455d0610"},{"author":{"_account_id":17821,"name":"Zhixin Li","email":"lizhixin@awcloud.com","username":"lizhixin"},"change_message_id":"904224975aa7bd39c983fa05a3a41e1cf248b5fd","unresolved":false,"context_lines":[{"line_number":348,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":349,"context_line":"                              router_id)"},{"line_number":350,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":351,"context_line":"                ri.delete()"},{"line_number":352,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def _safe_router_removed(self, router_id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa31d9ce_e64e82c0","line":351,"range":{"start_line":351,"start_character":15,"end_line":351,"end_character":27},"updated":"2017-02-10 12:23:27.000000000","message":"The ri.delete raises error and the save_and_reraise_exception doesn\u0027t work, so the exception info will come from ri.delete. It raises error due to invoking ha_port and keepalived_manager, which is empty or non-initialized. The ri.delete does nothing but raises error here, so I think the ri.delete should be deleted.","commit_id":"77b4ccb44b3dab314661a440c10cca193ccc5a06"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"47dbd4143a5cd4a89218f7fdde2ec60c1cd6fba6","unresolved":false,"context_lines":[{"line_number":348,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":349,"context_line":"                              router_id)"},{"line_number":350,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":351,"context_line":"                ri.delete()"},{"line_number":352,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def _safe_router_removed(self, router_id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa31d9ce_8c7327b1","line":351,"range":{"start_line":351,"start_character":15,"end_line":351,"end_character":27},"in_reply_to":"fa31d9ce_e64e82c0","updated":"2017-02-10 12:56:17.000000000","message":"Thanks. How should we cleanup any resources which are created in initialize()?","commit_id":"77b4ccb44b3dab314661a440c10cca193ccc5a06"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"4b1c4d083a1a10666aa16a6e199060598c031bae","unresolved":false,"context_lines":[{"line_number":340,"context_line":"        ri \u003d self._create_router(router_id, router)"},{"line_number":341,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE,"},{"line_number":342,"context_line":"                        self, router\u003dri)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"        ri.initialize(self.process_monitor)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa31d9ce_08f863c2","side":"PARENT","line":343,"updated":"2017-02-15 03:05:40.000000000","message":"nit: gratuitous whitespace change, L345 as well.","commit_id":"ff2c05142934dcd8ba786dab2e13cb3904d71d3f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"4b1c4d083a1a10666aa16a6e199060598c031bae","unresolved":false,"context_lines":[{"line_number":350,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":351,"context_line":"                              router_id)"},{"line_number":352,"context_line":"                try:"},{"line_number":353,"context_line":"                    self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":354,"context_line":"                    ri.delete()"},{"line_number":355,"context_line":"                except Exception:"},{"line_number":356,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa31d9ce_c8eacb73","line":353,"updated":"2017-02-15 03:05:40.000000000","message":"Is this doing to throw an error or just the delete?  Maybe doesn\u0027t need to be under the try.","commit_id":"4fde739008bdc1c4a74ee81afe4b6e0d49631113"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"524e846f8bff930e8ae1aaed17db24c99819ee0b","unresolved":false,"context_lines":[{"line_number":350,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":351,"context_line":"                              router_id)"},{"line_number":352,"context_line":"                try:"},{"line_number":353,"context_line":"                    self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":354,"context_line":"                    ri.delete()"},{"line_number":355,"context_line":"                except Exception:"},{"line_number":356,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa31d9ce_cc25785c","line":353,"in_reply_to":"fa31d9ce_c8eacb73","updated":"2017-02-15 07:40:29.000000000","message":"Doesn\u0027t throw an exception. I will move it.","commit_id":"4fde739008bdc1c4a74ee81afe4b6e0d49631113"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"4b1c4d083a1a10666aa16a6e199060598c031bae","unresolved":false,"context_lines":[{"line_number":351,"context_line":"                              router_id)"},{"line_number":352,"context_line":"                try:"},{"line_number":353,"context_line":"                    self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":354,"context_line":"                    ri.delete()"},{"line_number":355,"context_line":"                except Exception:"},{"line_number":356,"context_line":"                    pass"},{"line_number":357,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"fa31d9ce_88c293e7","line":354,"updated":"2017-02-15 03:05:40.000000000","message":"From the comment in PS3 this will raise an exception?  Although I guess you do have to call it to get whatever initialized un-done.","commit_id":"4fde739008bdc1c4a74ee81afe4b6e0d49631113"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"524e846f8bff930e8ae1aaed17db24c99819ee0b","unresolved":false,"context_lines":[{"line_number":351,"context_line":"                              router_id)"},{"line_number":352,"context_line":"                try:"},{"line_number":353,"context_line":"                    self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":354,"context_line":"                    ri.delete()"},{"line_number":355,"context_line":"                except Exception:"},{"line_number":356,"context_line":"                    pass"},{"line_number":357,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"fa31d9ce_acd92459","line":354,"in_reply_to":"fa31d9ce_88c293e7","updated":"2017-02-15 07:40:29.000000000","message":"Yes, it may raise exception. And I need to call it to undo whatever initialize() has done.","commit_id":"4fde739008bdc1c4a74ee81afe4b6e0d49631113"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"9bb280dfeca4818190f27be8979e1cda957f4121","unresolved":false,"context_lines":[{"line_number":346,"context_line":"        # If initialize() fails, cleanup and retrigger complete sync"},{"line_number":347,"context_line":"        try:"},{"line_number":348,"context_line":"            ri.initialize(self.process_monitor)"},{"line_number":349,"context_line":"        except Exception:"},{"line_number":350,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":351,"context_line":"                self.router_info.pop(router_id)"},{"line_number":352,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba2be162_12b41380","line":349,"range":{"start_line":349,"start_character":15,"end_line":349,"end_character":24},"updated":"2017-03-03 19:02:04.000000000","message":"Following on my comment at [1], it might be worth thinking about splitting up the 2 cases: if a NeutronException is thrown then we can resync and retry, and if an Exception is thrown then we... huh... quit?","commit_id":"ff29ec80ac9d659b1a30f061ba4b9c99dd96cf13"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"f0b9713af60cb2b45f207b7571cb78a5157fccbf","unresolved":false,"context_lines":[{"line_number":346,"context_line":"        # If initialize() fails, cleanup and retrigger complete sync"},{"line_number":347,"context_line":"        try:"},{"line_number":348,"context_line":"            ri.initialize(self.process_monitor)"},{"line_number":349,"context_line":"        except Exception:"},{"line_number":350,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":351,"context_line":"                self.router_info.pop(router_id)"},{"line_number":352,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a30ddce_0be074f9","line":349,"range":{"start_line":349,"start_character":15,"end_line":349,"end_character":24},"in_reply_to":"ba2be162_12b41380","updated":"2017-03-10 08:45:18.000000000","message":"That is a nice idea, but we need more discussion on retry for run time exceptions. But for now, shall we treat all exceptions initialize() throws as same and go ahead with retry for these exceptions(i.e existing code)?","commit_id":"ff29ec80ac9d659b1a30f061ba4b9c99dd96cf13"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"c38ef231c60d5eefeed9194151708e88cdeab9a7","unresolved":false,"context_lines":[{"line_number":355,"context_line":"                try:"},{"line_number":356,"context_line":"                    ri.delete()"},{"line_number":357,"context_line":"                except Exception:"},{"line_number":358,"context_line":"                    pass"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"    def _safe_router_removed(self, router_id):"},{"line_number":361,"context_line":"        \"\"\"Try to delete a router and return True if successful.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"da36d5c6_d93cff31","line":358,"range":{"start_line":358,"start_character":20,"end_line":358,"end_character":24},"updated":"2017-02-20 14:03:15.000000000","message":"some log message here as well?","commit_id":"ff29ec80ac9d659b1a30f061ba4b9c99dd96cf13"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"e339c238c64c06741ff9a498cd0ffd8daf9ff7b9","unresolved":false,"context_lines":[{"line_number":355,"context_line":"                try:"},{"line_number":356,"context_line":"                    ri.delete()"},{"line_number":357,"context_line":"                except Exception:"},{"line_number":358,"context_line":"                    pass"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"    def _safe_router_removed(self, router_id):"},{"line_number":361,"context_line":"        \"\"\"Try to delete a router and return True if successful.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"da36d5c6_f148f75d","line":358,"range":{"start_line":358,"start_character":20,"end_line":358,"end_character":24},"in_reply_to":"da36d5c6_d93cff31","updated":"2017-02-20 15:46:34.000000000","message":"sure, I will add. Thanks","commit_id":"ff29ec80ac9d659b1a30f061ba4b9c99dd96cf13"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"80ad774530055e7447bd1c1593bad4f03d97f17e","unresolved":false,"context_lines":[{"line_number":355,"context_line":"                try:"},{"line_number":356,"context_line":"                    ri.delete()"},{"line_number":357,"context_line":"                except Exception:"},{"line_number":358,"context_line":"                    pass"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"    def _safe_router_removed(self, router_id):"},{"line_number":361,"context_line":"        \"\"\"Try to delete a router and return True if successful.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba2be162_8901d5b1","line":358,"range":{"start_line":358,"start_character":20,"end_line":358,"end_character":24},"in_reply_to":"da36d5c6_f148f75d","updated":"2017-03-03 10:44:32.000000000","message":"go go go :D ;)","commit_id":"ff29ec80ac9d659b1a30f061ba4b9c99dd96cf13"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"5c7a69da8c8678aee0d85d4675715dbf6751933f","unresolved":false,"context_lines":[{"line_number":348,"context_line":"            ri.initialize(self.process_monitor)"},{"line_number":349,"context_line":"        except Exception:"},{"line_number":350,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":351,"context_line":"                self.router_info.pop(router_id)"},{"line_number":352,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":353,"context_line":"                              router_id)"},{"line_number":354,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a1ced50_016d4298","line":351,"updated":"2017-03-17 14:15:41.000000000","message":"since you\u0027re not using the return value, del self.router_info[router_id] is maybe better, but it\u0027s a nit since they\u0027re both accomplishing the same thing.","commit_id":"2adcd80dc4df87dc298fd3682cb687d410256c3a"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"1314ab13fcc6887ca84c7b55a53814a9792a21a6","unresolved":false,"context_lines":[{"line_number":348,"context_line":"            ri.initialize(self.process_monitor)"},{"line_number":349,"context_line":"        except Exception:"},{"line_number":350,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":351,"context_line":"                self.router_info.pop(router_id)"},{"line_number":352,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":353,"context_line":"                              router_id)"},{"line_number":354,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a1ced50_5ece0cb5","line":351,"in_reply_to":"1a1ced50_016d4298","updated":"2017-03-20 05:30:09.000000000","message":"Thanks Brian for fixing it","commit_id":"2adcd80dc4df87dc298fd3682cb687d410256c3a"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1bd3fc30b59c692c610c7cebcdaf6e4df21ed60f","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE,"},{"line_number":342,"context_line":"                        self, router\u003dri)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"        # If initialize() fails, cleanup and retrigger complete sync"},{"line_number":347,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a1ced50_1530b7e7","line":344,"updated":"2017-03-21 11:09:34.000000000","message":"+1, we can just move the assignment to after initialize and then we don\u0027t even need the \u0027del\u0027 on L351","commit_id":"d118f7c9fd95e1a6e5aabe48f0c8dfde701cfdf9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"5aa7366d2c7b1778e49cae0b9524613a66f97aea","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE,"},{"line_number":342,"context_line":"                        self, router\u003dri)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"        # If initialize() fails, cleanup and retrigger complete sync"},{"line_number":347,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a1ced50_d9d90324","line":344,"updated":"2017-03-21 08:07:09.000000000","message":"Should we add ri to self.router_info only if ri.initialize succeed?","commit_id":"d118f7c9fd95e1a6e5aabe48f0c8dfde701cfdf9"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3411f90dc4bfb8038fb524a160186b3db277a7b6","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE,"},{"line_number":342,"context_line":"                        self, router\u003dri)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"        # If initialize() fails, cleanup and retrigger complete sync"},{"line_number":347,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a1ced50_afda0be6","line":344,"in_reply_to":"1a1ced50_1530b7e7","updated":"2017-03-22 05:00:29.000000000","message":"sure","commit_id":"d118f7c9fd95e1a6e5aabe48f0c8dfde701cfdf9"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"88c45fc008ee952719e317bb5d49a7ff49eb278c","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE,"},{"line_number":342,"context_line":"                        self, router\u003dri)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"        # If initialize() fails, cleanup and retrigger complete sync"},{"line_number":347,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a1ced50_0011ee5c","line":344,"in_reply_to":"1a1ced50_afda0be6","updated":"2017-03-23 11:47:47.000000000","message":"No, this change is needed for [1] i.e notifying server for the router initial state change(to standby) during initialize. otherwise test_keepalived_state_change_bulk_rpc [2] test fails. \n\n[1] https://github.com/openstack/neutron/blob/master/neutron/agent/l3/ha.py#L120\n[2] http://logs.openstack.org/26/431026/11/check/gate-neutron-dsvm-functional-ubuntu-xenial/94f8095/testr_results.html.gz","commit_id":"d118f7c9fd95e1a6e5aabe48f0c8dfde701cfdf9"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1bd3fc30b59c692c610c7cebcdaf6e4df21ed60f","unresolved":false,"context_lines":[{"line_number":351,"context_line":"                del self.router_info[router_id]"},{"line_number":352,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":353,"context_line":"                              router_id)"},{"line_number":354,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":355,"context_line":"                try:"},{"line_number":356,"context_line":"                    ri.delete()"},{"line_number":357,"context_line":"                except Exception:"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a1ced50_354f736c","line":354,"updated":"2017-03-21 11:09:34.000000000","message":"+1, is there any chance this can fail?","commit_id":"d118f7c9fd95e1a6e5aabe48f0c8dfde701cfdf9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ac3ed98f36509e3cca490e819b9f585d50db797e","unresolved":false,"context_lines":[{"line_number":351,"context_line":"                del self.router_info[router_id]"},{"line_number":352,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":353,"context_line":"                              router_id)"},{"line_number":354,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":355,"context_line":"                try:"},{"line_number":356,"context_line":"                    ri.delete()"},{"line_number":357,"context_line":"                except Exception:"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a1ced50_5e5c4ced","line":354,"updated":"2017-03-20 05:36:48.000000000","message":"should this line be under the try?","commit_id":"d118f7c9fd95e1a6e5aabe48f0c8dfde701cfdf9"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3411f90dc4bfb8038fb524a160186b3db277a7b6","unresolved":false,"context_lines":[{"line_number":351,"context_line":"                del self.router_info[router_id]"},{"line_number":352,"context_line":"                LOG.exception(_LE(\u0027Error while initializing router %s\u0027),"},{"line_number":353,"context_line":"                              router_id)"},{"line_number":354,"context_line":"                self.namespaces_manager.ensure_router_cleanup(router_id)"},{"line_number":355,"context_line":"                try:"},{"line_number":356,"context_line":"                    ri.delete()"},{"line_number":357,"context_line":"                except Exception:"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a1ced50_0fc95f26","line":354,"in_reply_to":"1a1ced50_354f736c","updated":"2017-03-22 05:00:29.000000000","message":"I don\u0027t think so. In _router_removed() function [1] also this call is not under any try block. \n\nThis is discussed in PS5 https://review.openstack.org/#/c/431026/5/neutron/agent/l3/agent.py@353\n\n[1] https://github.com/openstack/neutron/blob/master/neutron/agent/l3/agent.py#L365","commit_id":"d118f7c9fd95e1a6e5aabe48f0c8dfde701cfdf9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e99fbd6798174ddb30e0730b03ebdedb259b13af","unresolved":false,"context_lines":[{"line_number":340,"context_line":"        ri \u003d self._create_router(router_id, router)"},{"line_number":341,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE,"},{"line_number":342,"context_line":"                        self, router\u003dri)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"        ri.initialize(self.process_monitor)"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a1ced50_09882dc6","side":"PARENT","line":343,"updated":"2017-03-23 16:04:59.000000000","message":"nit: unrelated whitespace change","commit_id":"f6546bcb1deab0d811c4cad968d2a176f2321e94"}],"neutron/agent/l3/ha_router.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"330d7a7394e20ebe99f472fe08c1798c25588f5d","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        if not ha_port:"},{"line_number":101,"context_line":"            LOG.error(_LE(\u0027Unable to process HA router %s without HA port\u0027),"},{"line_number":102,"context_line":"                      self.router_id)"},{"line_number":103,"context_line":"            raise Exception()"},{"line_number":104,"context_line":"        super(HaRouter, self).initialize(process_monitor)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        self.ha_port \u003d ha_port"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a430d35_aba657ab","line":103,"updated":"2017-02-08 16:18:38.000000000","message":"I think you can have the exception code print the message:\n\nmsg \u003d _(\"Unable ...\") % self.router_id\nraise Exception(msg)","commit_id":"d4fb916a3d18b5498fd81c068e2ccaf4b96d8713"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"25427c61062f55ae4ffd8e2a9ed0b48ebe83f079","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        if not ha_port:"},{"line_number":101,"context_line":"            LOG.error(_LE(\u0027Unable to process HA router %s without HA port\u0027),"},{"line_number":102,"context_line":"                      self.router_id)"},{"line_number":103,"context_line":"            raise Exception()"},{"line_number":104,"context_line":"        super(HaRouter, self).initialize(process_monitor)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        self.ha_port \u003d ha_port"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a430d35_ae3d85c5","line":103,"in_reply_to":"1a430d35_aba657ab","updated":"2017-02-08 16:30:40.000000000","message":"Thanks. Done","commit_id":"d4fb916a3d18b5498fd81c068e2ccaf4b96d8713"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"9bb280dfeca4818190f27be8979e1cda957f4121","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        if not ha_port:"},{"line_number":101,"context_line":"            msg \u003d _(\"Unable to process HA router %s without \""},{"line_number":102,"context_line":"                    \"HA port\") % self.router_id"},{"line_number":103,"context_line":"            raise Exception(msg)"},{"line_number":104,"context_line":"        super(HaRouter, self).initialize(process_monitor)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        self.ha_port \u003d ha_port"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba2be162_d2619b3a","line":103,"range":{"start_line":103,"start_character":18,"end_line":103,"end_character":27},"updated":"2017-03-03 19:02:04.000000000","message":"I would probably prefer to throw some kind of a NeutronException here, just to make sure it doesn\u0027t completely crash the process...\n\nHi :)","commit_id":"ff29ec80ac9d659b1a30f061ba4b9c99dd96cf13"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"f0b9713af60cb2b45f207b7571cb78a5157fccbf","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        if not ha_port:"},{"line_number":101,"context_line":"            msg \u003d _(\"Unable to process HA router %s without \""},{"line_number":102,"context_line":"                    \"HA port\") % self.router_id"},{"line_number":103,"context_line":"            raise Exception(msg)"},{"line_number":104,"context_line":"        super(HaRouter, self).initialize(process_monitor)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        self.ha_port \u003d ha_port"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a30ddce_ab904844","line":103,"range":{"start_line":103,"start_character":18,"end_line":103,"end_character":27},"in_reply_to":"ba2be162_d2619b3a","updated":"2017-03-10 08:45:18.000000000","message":"My comment in agent.py is also applicable here. Shall we just log here for now and later may be introduce new Neutron exception, if we want to have different handler code for this and other runtime exceptions.","commit_id":"ff29ec80ac9d659b1a30f061ba4b9c99dd96cf13"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ac3ed98f36509e3cca490e819b9f585d50db797e","unresolved":false,"context_lines":[{"line_number":101,"context_line":"            msg \u003d _(\"Unable to process HA router %s without \""},{"line_number":102,"context_line":"                    \"HA port\") % self.router_id"},{"line_number":103,"context_line":"            LOG.exception(msg)"},{"line_number":104,"context_line":"            raise Exception(msg)"},{"line_number":105,"context_line":"        super(HaRouter, self).initialize(process_monitor)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        self.ha_port \u003d ha_port"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a1ced50_be6d409a","line":104,"updated":"2017-03-20 05:36:48.000000000","message":"can we raise a more specific exception then you can catch this exception?","commit_id":"d118f7c9fd95e1a6e5aabe48f0c8dfde701cfdf9"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3411f90dc4bfb8038fb524a160186b3db277a7b6","unresolved":false,"context_lines":[{"line_number":101,"context_line":"            msg \u003d _(\"Unable to process HA router %s without \""},{"line_number":102,"context_line":"                    \"HA port\") % self.router_id"},{"line_number":103,"context_line":"            LOG.exception(msg)"},{"line_number":104,"context_line":"            raise Exception(msg)"},{"line_number":105,"context_line":"        super(HaRouter, self).initialize(process_monitor)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        self.ha_port \u003d ha_port"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a1ced50_6f7013dd","line":104,"in_reply_to":"1a1ced50_be6d409a","updated":"2017-03-22 05:00:29.000000000","message":"We don\u0027t need specific exception for now. We want to resync and retry router initialize if initialize() fails for any reason,  which this patch does. This is already discussed in [1] and [2]\n\n[1] https://review.openstack.org/#/c/431026/6/neutron/agent/l3/agent.py@349\n[2] https://review.openstack.org/#/c/431026/6/neutron/agent/l3/ha_router.py@103","commit_id":"d118f7c9fd95e1a6e5aabe48f0c8dfde701cfdf9"}],"neutron/tests/unit/agent/l3/test_agent.py":[{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"9bb280dfeca4818190f27be8979e1cda957f4121","unresolved":false,"context_lines":[{"line_number":1709,"context_line":"        self.assertEqual(1, ri.process.call_count)"},{"line_number":1710,"context_line":"        self.assertIn(ri.router_id, agent.router_info)"},{"line_number":1711,"context_line":""},{"line_number":1712,"context_line":"    def test_initialize_and_delete_fail_retrigger_initialize(self):"},{"line_number":1713,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"},{"line_number":1714,"context_line":"        router \u003d {\u0027id\u0027: _uuid(),"},{"line_number":1715,"context_line":"                  \u0027external_gateway_info\u0027: {\u0027network_id\u0027: \u0027aaa\u0027}}"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba2be162_123973f2","line":1712,"range":{"start_line":1712,"start_character":8,"end_line":1712,"end_character":60},"updated":"2017-03-03 19:02:04.000000000","message":"To me, this seems like the exact same test as test_initialize_fail_retrigger_initialize... what\u0027s different? If it is different - extract common code to shared function :)","commit_id":"ff29ec80ac9d659b1a30f061ba4b9c99dd96cf13"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"f0b9713af60cb2b45f207b7571cb78a5157fccbf","unresolved":false,"context_lines":[{"line_number":1709,"context_line":"        self.assertEqual(1, ri.process.call_count)"},{"line_number":1710,"context_line":"        self.assertIn(ri.router_id, agent.router_info)"},{"line_number":1711,"context_line":""},{"line_number":1712,"context_line":"    def test_initialize_and_delete_fail_retrigger_initialize(self):"},{"line_number":1713,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"},{"line_number":1714,"context_line":"        router \u003d {\u0027id\u0027: _uuid(),"},{"line_number":1715,"context_line":"                  \u0027external_gateway_info\u0027: {\u0027network_id\u0027: \u0027aaa\u0027}}"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a30ddce_ebe400e1","line":1712,"range":{"start_line":1712,"start_character":8,"end_line":1712,"end_character":60},"in_reply_to":"ba2be162_123973f2","updated":"2017-03-10 08:45:18.000000000","message":"I will extract. Thanks :)","commit_id":"ff29ec80ac9d659b1a30f061ba4b9c99dd96cf13"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"618c5ca3605b48bce33fb11561e6dd4dcd69cea6","unresolved":false,"context_lines":[{"line_number":1739,"context_line":"        agent._create_router \u003d mock.Mock(return_value\u003dri)"},{"line_number":1740,"context_line":"        agent._process_router_update()"},{"line_number":1741,"context_line":"        agent._process_router_update()"},{"line_number":1742,"context_line":"        self.assertTrue(ri.delete.called)"},{"line_number":1743,"context_line":"        self.assertEqual(2, ri.initialize.call_count)"},{"line_number":1744,"context_line":"        self.assertEqual(2, agent._create_router.call_count)"},{"line_number":1745,"context_line":"        self.assertEqual(1, ri.process.call_count)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ffe62b97_1724c6c5","line":1742,"updated":"2017-03-23 23:34:54.000000000","message":"For the case where delete_fail\u003dTrue, why not also check that the exception is properly logged. Here\u0027s an example:\n\nhttps://github.com/openstack/neutron/blob/master/neutron/tests/unit/extensions/test_segment.py#L1725","commit_id":"0179fdb65914b51c9c24940535e27cc5fb78950a"}]}
