)]}'
{"swift/cli/relinker.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d9011a36fc83c41e2a0f3bb76fdc9597662fe879","unresolved":true,"context_lines":[{"line_number":121,"context_line":"        self._update_recon(force_dump\u003dTrue)"},{"line_number":122,"context_line":"        self.logger.warning(\"Signal %d received. Exiting with return_code %d\","},{"line_number":123,"context_line":"                            sig, self.return_code)"},{"line_number":124,"context_line":"        os.exit(self.return_code)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def _update_recon(self, device\u003dNone, force_dump\u003dFalse):"},{"line_number":127,"context_line":"        if not force_dump and self._last_recon_update + self.recon_interval \\"}],"source_content_type":"text/x-python","patch_set":2,"id":"3d7a521c_cec74ebc","line":124,"updated":"2021-04-28 07:29:34.000000000","message":"The nuances between os.exit and os._exit really threw me this arvo... Fun times :P\n\nBut seeing as the signal is telling it to all close up, the extra work in os.exit seems to make it all behave properly, especially when trying to test it all.","commit_id":"d9a7b3b6c9d4e182aeb696a4dfc2bee5f046c98c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d9011a36fc83c41e2a0f3bb76fdc9597662fe879","unresolved":true,"context_lines":[{"line_number":654,"context_line":"                linked, removed, action_errors + listdir_errors)"},{"line_number":655,"context_line":"            self._update_worker_stats()"},{"line_number":656,"context_line":"        finally:"},{"line_number":657,"context_line":"            return self.return_code"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":""},{"line_number":660,"context_line":"def _reset_recon(recon_cache, logger):"}],"source_content_type":"text/x-python","patch_set":2,"id":"25919006_24246995","line":657,"updated":"2021-04-28 07:29:34.000000000","message":"This is letting us return the right return code when we exit from a signal handler, without it I could never get EXIT_SIGNAL to return properly in the tests.\n\nMaybe I just need fresh eyes, so will look again in the morning.","commit_id":"d9a7b3b6c9d4e182aeb696a4dfc2bee5f046c98c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"84f57f772d87c902a0962fcbc37cb43b1c793fd5","unresolved":true,"context_lines":[{"line_number":122,"context_line":"        signal.signal(signal.SIGTERM, self._signal_handler)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _signal_handler(self, sig, frame):"},{"line_number":125,"context_line":"        self.return_code \u003d EXIT_SIGNAL"},{"line_number":126,"context_line":"        self._update_recon(force_dump\u003dTrue)"},{"line_number":127,"context_line":"        self.logger.warning(\"Signal %d received. Exiting with return_code %d\","},{"line_number":128,"context_line":"                            sig, self.return_code)"}],"source_content_type":"text/x-python","patch_set":6,"id":"0b24c78a_7ee3c374","line":125,"updated":"2021-05-05 22:11:43.000000000","message":"I think I\u0027d be OK with using EXIT_ERROR -- it exited abnormally, so we have to assume there\u0027s still work to be done.\n\nOr -- we\u0027ve already got some handling for workers exiting after receiving a signal... what if we do something like\n\n signal.signal(sig, signal.SIG_DFL)\n os.kill(os.getpid(), sig)\n\n?","commit_id":"58559f2632320c20cace0fec33bbc6e263effcc4"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"3825fe6ec7ca68282993140423e1a7ef90a33da3","unresolved":true,"context_lines":[{"line_number":122,"context_line":"        signal.signal(signal.SIGTERM, self._signal_handler)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _signal_handler(self, sig, frame):"},{"line_number":125,"context_line":"        self.return_code \u003d EXIT_SIGNAL"},{"line_number":126,"context_line":"        self._update_recon(force_dump\u003dTrue)"},{"line_number":127,"context_line":"        self.logger.warning(\"Signal %d received. Exiting with return_code %d\","},{"line_number":128,"context_line":"                            sig, self.return_code)"}],"source_content_type":"text/x-python","patch_set":6,"id":"20adac90_63639535","line":125,"in_reply_to":"0b24c78a_7ee3c374","updated":"2021-05-11 07:01:16.000000000","message":"oh yeah, so daemon is killing the process group. Ie, os.killpg(0) which must map to the one it\u0027s running in.\n\nI guess the question is do we want to kill all children/members of the pg on a signal, or allow us to signal individual processes.","commit_id":"58559f2632320c20cace0fec33bbc6e263effcc4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"84f57f772d87c902a0962fcbc37cb43b1c793fd5","unresolved":true,"context_lines":[{"line_number":124,"context_line":"    def _signal_handler(self, sig, frame):"},{"line_number":125,"context_line":"        self.return_code \u003d EXIT_SIGNAL"},{"line_number":126,"context_line":"        self._update_recon(force_dump\u003dTrue)"},{"line_number":127,"context_line":"        self.logger.warning(\"Signal %d received. Exiting with return_code %d\","},{"line_number":128,"context_line":"                            sig, self.return_code)"},{"line_number":129,"context_line":"        os.exit(self.return_code)"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"12a66354_c26a4ae9","line":127,"updated":"2021-05-05 22:11:43.000000000","message":"I\u0027m wary of logging inside a signal handler -- there\u0027s a lock used throughout logging that could cause a deadlock if it\u0027s held by something other than the main thread when the signal gets processed.\n\nOTOH, this should all be single-threaded, right? Likely have multiple processes, but each should have just the one thread...","commit_id":"58559f2632320c20cace0fec33bbc6e263effcc4"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d6b302568f42339af90216e229b3d9fd518bec17","unresolved":true,"context_lines":[{"line_number":124,"context_line":"    def _signal_handler(self, sig, frame):"},{"line_number":125,"context_line":"        self.return_code \u003d EXIT_SIGNAL"},{"line_number":126,"context_line":"        self._update_recon(force_dump\u003dTrue)"},{"line_number":127,"context_line":"        self.logger.warning(\"Signal %d received. Exiting with return_code %d\","},{"line_number":128,"context_line":"                            sig, self.return_code)"},{"line_number":129,"context_line":"        os.exit(self.return_code)"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"b88df658_d409f475","line":127,"in_reply_to":"12a66354_c26a4ae9","updated":"2021-05-11 06:16:31.000000000","message":"yeah as it currenty stands because we\u0027re using fork they all should be extra signle threaded processes right. Although yeah, happy to remove the logging if it\u0027s an issue.","commit_id":"58559f2632320c20cace0fec33bbc6e263effcc4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"84f57f772d87c902a0962fcbc37cb43b1c793fd5","unresolved":true,"context_lines":[{"line_number":126,"context_line":"        self._update_recon(force_dump\u003dTrue)"},{"line_number":127,"context_line":"        self.logger.warning(\"Signal %d received. Exiting with return_code %d\","},{"line_number":128,"context_line":"                            sig, self.return_code)"},{"line_number":129,"context_line":"        os.exit(self.return_code)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def _aggregate_dev_policy_stats(self):"},{"line_number":132,"context_line":"        for dev_data in self.devices_data.values():"}],"source_content_type":"text/x-python","patch_set":6,"id":"ef2cd453_dcd273a9","line":129,"updated":"2021-05-05 22:11:43.000000000","message":"This is a pretty different approach to signal handlers compared to what we do in wsgi and daemon, where we might set a flag or signal children then get out. Not saying it\u0027s necessarily bad, just want to think through the reason we do it that way currently and whether it\u0027d be better for us to follow precedent.\n\nIf we really want to kill ourselves from the signal handler, I think we might need to use os._exit() so unit tests can pass.","commit_id":"58559f2632320c20cace0fec33bbc6e263effcc4"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d6b302568f42339af90216e229b3d9fd518bec17","unresolved":true,"context_lines":[{"line_number":126,"context_line":"        self._update_recon(force_dump\u003dTrue)"},{"line_number":127,"context_line":"        self.logger.warning(\"Signal %d received. Exiting with return_code %d\","},{"line_number":128,"context_line":"                            sig, self.return_code)"},{"line_number":129,"context_line":"        os.exit(self.return_code)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def _aggregate_dev_policy_stats(self):"},{"line_number":132,"context_line":"        for dev_data in self.devices_data.values():"}],"source_content_type":"text/x-python","patch_set":6,"id":"b71eeff9_78297ee6","line":129,"in_reply_to":"ef2cd453_dcd273a9","updated":"2021-05-11 06:16:31.000000000","message":"Fair enough, yeah I\u0027ll have a look at what were doing over there.\n\nActually I needed to change this from os._exit() to os.exit() so the tests would actaully behave themselves. os._exit() seems to kill straight away with no cleanup. Where as os.exit() behaved correctly so it could be caught and run through the finally.","commit_id":"58559f2632320c20cace0fec33bbc6e263effcc4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"84f57f772d87c902a0962fcbc37cb43b1c793fd5","unresolved":true,"context_lines":[{"line_number":675,"context_line":"            self._update_worker_stats()"},{"line_number":676,"context_line":"        except IOError as err:"},{"line_number":677,"context_line":"            self.return_code \u003d EXIT_ERROR"},{"line_number":678,"context_line":"            self.logger.warning(\u0027Relinker failed with IO Error: %s\u0027, err)"},{"line_number":679,"context_line":"            self._update_worker_stats()"},{"line_number":680,"context_line":"        finally:"},{"line_number":681,"context_line":"            return self.return_code"}],"source_content_type":"text/x-python","patch_set":6,"id":"e68d15d0_9bb6f761","line":678,"updated":"2021-05-05 22:11:43.000000000","message":"We\u0027ve lost the traceback -- have we seen IOErrors popping that bomb out the whole process? Where did they bubble up from?","commit_id":"58559f2632320c20cace0fec33bbc6e263effcc4"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d6b302568f42339af90216e229b3d9fd518bec17","unresolved":true,"context_lines":[{"line_number":675,"context_line":"            self._update_worker_stats()"},{"line_number":676,"context_line":"        except IOError as err:"},{"line_number":677,"context_line":"            self.return_code \u003d EXIT_ERROR"},{"line_number":678,"context_line":"            self.logger.warning(\u0027Relinker failed with IO Error: %s\u0027, err)"},{"line_number":679,"context_line":"            self._update_worker_stats()"},{"line_number":680,"context_line":"        finally:"},{"line_number":681,"context_line":"            return self.return_code"}],"source_content_type":"text/x-python","patch_set":6,"id":"7e5701b4_54fc2ff0","line":678,"in_reply_to":"e68d15d0_9bb6f761","updated":"2021-05-11 06:16:31.000000000","message":"oh really this is my bad.. it\u0027s bubling up from tests, I was firing them to test signals we don\u0027t catch. Maybe I just need to mock some more so I can bail cleaner.","commit_id":"58559f2632320c20cace0fec33bbc6e263effcc4"}],"test/unit/cli/test_relinker.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d9011a36fc83c41e2a0f3bb76fdc9597662fe879","unresolved":true,"context_lines":[{"line_number":1116,"context_line":"    @mock.patch(\u0027os.getpid\u0027, return_value\u003d100)"},{"line_number":1117,"context_line":"    def test_end_with_signal(self, mock_getpid):"},{"line_number":1118,"context_line":"        def mock_pre_dev(r, *args):"},{"line_number":1119,"context_line":"            signal.raise_signal(signal.SIGTERM)"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"        self.rb.prepare_increase_partition_power()"},{"line_number":1122,"context_line":"        self._save_ring()"}],"source_content_type":"text/x-python","patch_set":2,"id":"e48b8ee6_7ecfc5af","line":1119,"updated":"2021-04-28 07:29:34.000000000","message":"Now that I\u0027m able to actually raise signals without terminating the test suite (thanks os._exit) we can probably extend this to test some other signals that doesn\u0027t cause it to return 3. But wanted to get something that at least works off my computer at the end of my day.","commit_id":"d9a7b3b6c9d4e182aeb696a4dfc2bee5f046c98c"}]}