)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"574f24a9abcdfadfa5062be96e7491ec5430618b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"24d84407_16ae8e9d","updated":"2023-09-20 08:12:24.000000000","message":"recheck\ntest_get_datapath_id failure in functional is not related, opensearch link:\nhttps://opensearch.logs.openstack.org/_dashboards/app/discover/?security_tenant\u003dglobal#/?_g\u003d(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:now-30d,to:now))\u0026_a\u003d(columns:!(build_patchset,build_change,build_name,build_status),filters:!(),index:\u002794869730-aea8-11ec-9e6a-83741af3fdcd\u0027,interval:auto,query:(language:kuery,query:\u0027message:%22line%20222,%20in%20test_get_datapath_id%22\u0027),sort:!())","commit_id":"4ea323f71608c5af71278ad488f9c05096aed29e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"21f67f0ba4c2f9113471f4a3bbe585df4c427605","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"33c73b83_ca898999","updated":"2023-09-22 01:42:23.000000000","message":"recheck unrelated fullstack failure","commit_id":"4ea323f71608c5af71278ad488f9c05096aed29e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"fe410e83cd362740fa29fdd6d8595e62d90b911e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e541e9f9_15b46c1b","updated":"2023-09-21 19:03:02.000000000","message":"recheck unrelated fullstack failure","commit_id":"4ea323f71608c5af71278ad488f9c05096aed29e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"b8f3579616c5e47af62da2a5eb272ac59aef6a4a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fb0182f9_442d6683","updated":"2023-09-24 20:44:56.000000000","message":"recheck unrelated ha router failure","commit_id":"4ea323f71608c5af71278ad488f9c05096aed29e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"96912363cdc9f4facf8f1b7618a41ef6d55d57a6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"63ef08a0_eb87c71b","updated":"2023-09-23 23:19:21.000000000","message":"recheck unrelated tempest failure","commit_id":"4ea323f71608c5af71278ad488f9c05096aed29e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"fcca194af2bede5b1bcd582558c6f1a11081c22b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ea27b82a_dbadec09","updated":"2023-09-24 16:51:20.000000000","message":"recheck unrelated test failures","commit_id":"4ea323f71608c5af71278ad488f9c05096aed29e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2f78ea2c1151f47cd341a8a5205480ac5a1be52e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"7c38a811_3c8eba1e","updated":"2023-10-17 13:49:30.000000000","message":"-1 for the unit tests and the question inline.","commit_id":"157a21803745183881854abb36f4b80a101d8d69"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1fc4be5b42deed5694b2039ff381a87bcc3bbc91","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"84f3d206_663e002e","updated":"2023-10-17 18:18:20.000000000","message":"Unit test failures were from a missing mock, strange it didn\u0027t fail for me when run locally.","commit_id":"157a21803745183881854abb36f4b80a101d8d69"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"5a75b1268ffd4635a9f2f27b7cf2f2542109ecce","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"74e3d4d6_2b01b602","updated":"2023-10-27 18:41:11.000000000","message":"Last PS should have addresses Sven\u0027s concern.","commit_id":"c3b855a10080ab5b7d33f42aaee02e5ed50a4fdf"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"5b71cf46b59337673b1dbc79416046d1bb6306dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e9742606_37f9c0d6","updated":"2023-11-13 13:28:14.000000000","message":"Thanks for the patch and for the reviewers","commit_id":"c3b855a10080ab5b7d33f42aaee02e5ed50a4fdf"}],"neutron/agent/linux/external_process.py":[{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"5d7674434b9d06aab1e6f7be76a47194c79afc6e","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            # Always try and remove the pid file, as it\u0027s existence could"},{"line_number":91,"context_line":"            # stop the process from starting"},{"line_number":92,"context_line":"            pid_file \u003d self.get_pid_file_name()"},{"line_number":93,"context_line":"            utils.delete_if_exists(pid_file, run_as_root\u003dself.run_as_root)"},{"line_number":94,"context_line":"            cmd \u003d cmd_callback(pid_file)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"            ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dself.namespace)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1e236fa2_24a0570f","line":93,"range":{"start_line":93,"start_character":0,"end_line":93,"end_character":1},"updated":"2023-10-18 08:00:25.000000000","message":"none of the functions inside neutron called here (utils.delete_if_exists, privileged or not, handle any errors, but oslo.fileutils.delete_if_exists will raise every error besides ENOENT (file|dir does not exist).\n\nthus, these errors imho need to be handled somewhere, probably in `utils.py`.\n\nthere are many possible reasons why a file can\u0027t be deleted.\n\nthis would at least not print terrible stack traces in logs and be more user friendly 😊","commit_id":"5514b9c1375c4706c1db6cc49d9c37fce62896d1"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"96c668056129c63129c816311c316f45bf20c994","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            # Always try and remove the pid file, as it\u0027s existence could"},{"line_number":91,"context_line":"            # stop the process from starting"},{"line_number":92,"context_line":"            pid_file \u003d self.get_pid_file_name()"},{"line_number":93,"context_line":"            utils.delete_if_exists(pid_file, run_as_root\u003dself.run_as_root)"},{"line_number":94,"context_line":"            cmd \u003d cmd_callback(pid_file)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"            ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dself.namespace)"}],"source_content_type":"text/x-python","patch_set":4,"id":"38bd9367_92b67472","line":93,"range":{"start_line":93,"start_character":0,"end_line":93,"end_character":1},"in_reply_to":"1e236fa2_24a0570f","updated":"2023-10-19 12:34:51.000000000","message":"utils.delete_if_exists finally also calls fileutils.delete_if_exists so I don\u0027t see the issue here.","commit_id":"5514b9c1375c4706c1db6cc49d9c37fce62896d1"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1e282d2dc6d1fbcf1d48f53e3a3fd098d564c21a","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            # Always try and remove the pid file, as it\u0027s existence could"},{"line_number":91,"context_line":"            # stop the process from starting"},{"line_number":92,"context_line":"            pid_file \u003d self.get_pid_file_name()"},{"line_number":93,"context_line":"            utils.delete_if_exists(pid_file, run_as_root\u003dself.run_as_root)"},{"line_number":94,"context_line":"            cmd \u003d cmd_callback(pid_file)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"            ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dself.namespace)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7ec52306_e989e8a1","line":93,"range":{"start_line":93,"start_character":0,"end_line":93,"end_character":1},"in_reply_to":"322d14bf_ba0d6f59","updated":"2023-10-19 22:05:01.000000000","message":"This change actually doesn\u0027t alter how things work today - if when trying to remove a pid file and we get an ENOENT nothing is raised, anything other will raise. The only thing changed is it is now done in enable() and not just disable().\n\nOf course there a few of things we can do:\n\n1) Change every caller of enable() to try/except, which is pretty invasive and would need to be done in another patch as each caller would need to be evaluated.\n\n2) Add a try/except here, print a message and return early. Most callers will be registering a process monitor, so it will trigger a restart, which might just fail in the same way again and again.\n\n3) Add a try/except here, print a message and continue on to execute(). Will it succeed? Unknown, but I guess the error might help debug? The exception would have had that in the existing code too.\n\nI don\u0027t mind doing #3 above, but the solution for an admin still might require stopping the agent, fixing the issue, then starting it. Not much of a step forward.","commit_id":"5514b9c1375c4706c1db6cc49d9c37fce62896d1"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"544d53be2b4a19e003213c1b31bc2fd4a6e5a157","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            # Always try and remove the pid file, as it\u0027s existence could"},{"line_number":91,"context_line":"            # stop the process from starting"},{"line_number":92,"context_line":"            pid_file \u003d self.get_pid_file_name()"},{"line_number":93,"context_line":"            utils.delete_if_exists(pid_file, run_as_root\u003dself.run_as_root)"},{"line_number":94,"context_line":"            cmd \u003d cmd_callback(pid_file)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"            ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dself.namespace)"}],"source_content_type":"text/x-python","patch_set":4,"id":"322d14bf_ba0d6f59","line":93,"range":{"start_line":93,"start_character":0,"end_line":93,"end_character":1},"in_reply_to":"38bd9367_92b67472","updated":"2023-10-19 13:53:10.000000000","message":"???\n\nthat also doesn\u0027t handle any errors, beside file existence, see my original criticism above:\n\nhttps://github.com/openstack/oslo.utils/blob/cecf061e6e4f666104170e3bba93fdae24255309/oslo_utils/fileutils.py#L60\n\nIt just reraises the exception - a working approach for library code - to the calling program which needs to decide what it does with this raised exception:\n\nhttps://docs.python.org/3/tutorial/errors.html#raising-exceptions\n\nneutron currently does nothing with this and afaik just will write a stacktrace into it\u0027s logfile.\n\nwhich can be argued to be \"okay\", but is clearly not a very good user experience.\nI\u0027m talking from experience parsing way to many python stacktraces in various openstack logfiles.\n\nI was just trying to improve operators life by catching possible errors and print a nice error message instead, e.g. \"couldn\u0027t delete pid file because [..]\".\n\nIf you don\u0027t want to improve the error handling or consider it out of scope for this change, that\u0027s something I can\u0027t change.\n\nBut the current code clearly does not handle errors well.\n\nThanks","commit_id":"5514b9c1375c4706c1db6cc49d9c37fce62896d1"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"5a75b1268ffd4635a9f2f27b7cf2f2542109ecce","unresolved":false,"context_lines":[{"line_number":90,"context_line":"            # Always try and remove the pid file, as it\u0027s existence could"},{"line_number":91,"context_line":"            # stop the process from starting"},{"line_number":92,"context_line":"            pid_file \u003d self.get_pid_file_name()"},{"line_number":93,"context_line":"            utils.delete_if_exists(pid_file, run_as_root\u003dself.run_as_root)"},{"line_number":94,"context_line":"            cmd \u003d cmd_callback(pid_file)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"            ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dself.namespace)"}],"source_content_type":"text/x-python","patch_set":4,"id":"13a75ec6_471862a9","line":93,"range":{"start_line":93,"start_character":0,"end_line":93,"end_character":1},"in_reply_to":"64b5616a_b7315263","updated":"2023-10-27 18:41:11.000000000","message":"Done","commit_id":"5514b9c1375c4706c1db6cc49d9c37fce62896d1"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"40c34c359dfe3d8106bdfb04a9177defe4a119c4","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            # Always try and remove the pid file, as it\u0027s existence could"},{"line_number":91,"context_line":"            # stop the process from starting"},{"line_number":92,"context_line":"            pid_file \u003d self.get_pid_file_name()"},{"line_number":93,"context_line":"            utils.delete_if_exists(pid_file, run_as_root\u003dself.run_as_root)"},{"line_number":94,"context_line":"            cmd \u003d cmd_callback(pid_file)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"            ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dself.namespace)"}],"source_content_type":"text/x-python","patch_set":4,"id":"64b5616a_b7315263","line":93,"range":{"start_line":93,"start_character":0,"end_line":93,"end_character":1},"in_reply_to":"7ec52306_e989e8a1","updated":"2023-10-26 12:16:14.000000000","message":"\u003e This change actually doesn\u0027t alter how things work today[..]\n\nsure, that\u0027s exactly my criticism, new code should be better, than old code imho.\n\nso I\u0027d be fine with 3)","commit_id":"5514b9c1375c4706c1db6cc49d9c37fce62896d1"}],"neutron/agent/linux/ra.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"fd859a1681adfffb926f33f37aec12510c8e940a","unresolved":true,"context_lines":[{"line_number":189,"context_line":"        pm.enable(reload_cfg\u003dTrue)"},{"line_number":190,"context_line":"        self._process_monitor.register(uuid\u003dself._router_id,"},{"line_number":191,"context_line":"                                       service_name\u003dRADVD_SERVICE_NAME,"},{"line_number":192,"context_line":"                                       monitored_process\u003dpm)"},{"line_number":193,"context_line":"        LOG.debug(\"radvd enabled for router %s\", self._router_id)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def enable(self, router_ports):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1573184d_7fe0771f","line":192,"range":{"start_line":192,"start_character":39,"end_line":192,"end_character":59},"updated":"2023-09-25 08:54:57.000000000","message":"I would make this improvement more generic, for any \"ProcessManager\" instance. Before starting any process [1]. I would first remove the PID (same as you\u0027ve done here) and then I\u0027ll start the process.\n\n[1]https://github.com/openstack/neutron/blob/8e38e57b8000ca6ce9ab84692a9aba6220556a3d/neutron/agent/linux/external_process.py#L87-L94","commit_id":"4ea323f71608c5af71278ad488f9c05096aed29e"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"49185005b2f65b29c82b1bc0b5ba52b5d760ac44","unresolved":true,"context_lines":[{"line_number":189,"context_line":"        pm.enable(reload_cfg\u003dTrue)"},{"line_number":190,"context_line":"        self._process_monitor.register(uuid\u003dself._router_id,"},{"line_number":191,"context_line":"                                       service_name\u003dRADVD_SERVICE_NAME,"},{"line_number":192,"context_line":"                                       monitored_process\u003dpm)"},{"line_number":193,"context_line":"        LOG.debug(\"radvd enabled for router %s\", self._router_id)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def enable(self, router_ports):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ca7d324_c2b3cda7","line":192,"range":{"start_line":192,"start_character":39,"end_line":192,"end_character":59},"in_reply_to":"1573184d_7fe0771f","updated":"2023-09-26 10:27:49.000000000","message":"agreed, this seems to be a better more universal solution.","commit_id":"4ea323f71608c5af71278ad488f9c05096aed29e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c8009cd9baed9893a82635f566cd56ecbbe2a4e1","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        pm.enable(reload_cfg\u003dTrue)"},{"line_number":190,"context_line":"        self._process_monitor.register(uuid\u003dself._router_id,"},{"line_number":191,"context_line":"                                       service_name\u003dRADVD_SERVICE_NAME,"},{"line_number":192,"context_line":"                                       monitored_process\u003dpm)"},{"line_number":193,"context_line":"        LOG.debug(\"radvd enabled for router %s\", self._router_id)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def enable(self, router_ports):"}],"source_content_type":"text/x-python","patch_set":2,"id":"cbe4123a_02a2526b","line":192,"range":{"start_line":192,"start_character":39,"end_line":192,"end_character":59},"in_reply_to":"5ca7d324_c2b3cda7","updated":"2023-10-16 22:28:29.000000000","message":"Done","commit_id":"4ea323f71608c5af71278ad488f9c05096aed29e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2f78ea2c1151f47cd341a8a5205480ac5a1be52e","unresolved":true,"context_lines":[{"line_number":153,"context_line":"            run_as_root\u003dTrue)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    @staticmethod"},{"line_number":156,"context_line":"    def _safe_remove_pid_file(pid_file):"},{"line_number":157,"context_line":"        try:"},{"line_number":158,"context_line":"            os.remove(pid_file)"},{"line_number":159,"context_line":"        except OSError as e:"},{"line_number":160,"context_line":"            if e.errno !\u003d errno.ENOENT:"},{"line_number":161,"context_line":"                LOG.error(\"Could not delete file %s, radvd can \""},{"line_number":162,"context_line":"                          \"refuse to start.\", pid_file)"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def _spawn_radvd(self, radvd_conf):"},{"line_number":165,"context_line":"        def callback(pid_file):"}],"source_content_type":"text/x-python","patch_set":3,"id":"eda90c52_21612c17","line":162,"range":{"start_line":156,"start_character":4,"end_line":162,"end_character":55},"updated":"2023-10-17 13:49:30.000000000","message":"Is this a leftover from a previous PS?","commit_id":"157a21803745183881854abb36f4b80a101d8d69"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1fc4be5b42deed5694b2039ff381a87bcc3bbc91","unresolved":false,"context_lines":[{"line_number":153,"context_line":"            run_as_root\u003dTrue)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    @staticmethod"},{"line_number":156,"context_line":"    def _safe_remove_pid_file(pid_file):"},{"line_number":157,"context_line":"        try:"},{"line_number":158,"context_line":"            os.remove(pid_file)"},{"line_number":159,"context_line":"        except OSError as e:"},{"line_number":160,"context_line":"            if e.errno !\u003d errno.ENOENT:"},{"line_number":161,"context_line":"                LOG.error(\"Could not delete file %s, radvd can \""},{"line_number":162,"context_line":"                          \"refuse to start.\", pid_file)"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def _spawn_radvd(self, radvd_conf):"},{"line_number":165,"context_line":"        def callback(pid_file):"}],"source_content_type":"text/x-python","patch_set":3,"id":"147a2037_b09744ec","line":162,"range":{"start_line":156,"start_character":4,"end_line":162,"end_character":55},"in_reply_to":"eda90c52_21612c17","updated":"2023-10-17 18:18:20.000000000","message":"Yes, from previous PS and forgot to remove.","commit_id":"157a21803745183881854abb36f4b80a101d8d69"}]}