)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2de7b267baea947b6081219edada4f9d0aed9396","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"0c64f08c_952cc576","updated":"2024-11-13 21:25:39.000000000","message":"Yeah makes sense. All this really does is adds an actual value to timeout on probetests running `subprocess.Popen.wait` which by default is None. And you\u0027ve found a bug where these can freeze. This is a probe test and you\u0027ve proven it\u0027s alot more stable when we do.\n\nWe will need to rememeber about this timeout incase we ever do have longer runnning probe tests daemons. Could we make it configurable via the test.conf? So we have some control out of it. Otherwise, I\u0027m totally on board with this!","commit_id":"e7a1f7c65a7d376d68814321cf2f9d4d119db5e3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"1134fc9200cb20a882736c60b2feef5c63d78d05","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"041edd64_b2d657b8","in_reply_to":"0c64f08c_952cc576","updated":"2024-11-13 22:09:34.000000000","message":"Solid plan -- I think this should do.\n\nAnother thing that seems reasonably likely if we\u0027re anticipating a longer-running test: call `start()`, then monitor for success (up to some per-test timeout), then call `stop()` -- like we already do for `test_replication_servers_working.py`","commit_id":"e7a1f7c65a7d376d68814321cf2f9d4d119db5e3"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a5ed5f654aef75c33d2a7bce024f4ba1bc42efe0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"208e7c73_8f745208","updated":"2024-11-13 23:00:34.000000000","message":"yup love it!","commit_id":"2cc56922edd07ff4169af29cff13a3acb2d1fd8f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"403383796da5916b4a68beb9465d448a3bd686ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f6dda46e_4dce347d","updated":"2024-11-14 11:00:00.000000000","message":"I pushed the changes I needed to get this to kill and fail an aberrant test.\n\nI think it would be worth considering a different conf name.\n\nI can\u0027t decide how much I care about where the patching is done.","commit_id":"3ae9ed1915b7af692d67eb4aa33ed1f23fd16c94"}],"test/probe/__init__.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c3a720191fd89b217f3be0395a6e786007df317","unresolved":true,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"config \u003d get_config(\u0027probe_test\u0027)"},{"line_number":26,"context_line":"CHECK_SERVER_TIMEOUT \u003d int(config.get(\u0027check_server_timeout\u0027, 30))"},{"line_number":27,"context_line":"ONCE_TIMEOUT \u003d int(config.get(\u0027once_timeout\u0027, CHECK_SERVER_TIMEOUT))"},{"line_number":28,"context_line":"VALIDATE_RSYNC \u003d config_true_value(config.get(\u0027validate_rsync\u0027, False))"},{"line_number":29,"context_line":"PROXY_BASE_URL \u003d config.get(\u0027proxy_base_url\u0027)"},{"line_number":30,"context_line":"if PROXY_BASE_URL is None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"c8df0bd2_43a2f9a4","line":27,"range":{"start_line":27,"start_character":31,"end_line":27,"end_character":43},"updated":"2024-11-14 10:54:40.000000000","message":"the timeout is actually applied to more than just Manager.once() processes (e.g. rsync) so it might be better to call it what it is e.g. ``subprocess_wait_timeout``","commit_id":"2cc56922edd07ff4169af29cff13a3acb2d1fd8f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"43829bdcfe8e0c1edea956981e506abaef953e1c","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"config \u003d get_config(\u0027probe_test\u0027)"},{"line_number":26,"context_line":"CHECK_SERVER_TIMEOUT \u003d int(config.get(\u0027check_server_timeout\u0027, 30))"},{"line_number":27,"context_line":"ONCE_TIMEOUT \u003d int(config.get(\u0027once_timeout\u0027, CHECK_SERVER_TIMEOUT))"},{"line_number":28,"context_line":"VALIDATE_RSYNC \u003d config_true_value(config.get(\u0027validate_rsync\u0027, False))"},{"line_number":29,"context_line":"PROXY_BASE_URL \u003d config.get(\u0027proxy_base_url\u0027)"},{"line_number":30,"context_line":"if PROXY_BASE_URL is None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"4d485761_7d1d511b","line":27,"range":{"start_line":27,"start_character":31,"end_line":27,"end_character":43},"in_reply_to":"c8df0bd2_43a2f9a4","updated":"2024-11-14 18:35:47.000000000","message":"Done","commit_id":"2cc56922edd07ff4169af29cff13a3acb2d1fd8f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c3a720191fd89b217f3be0395a6e786007df317","unresolved":true,"context_lines":[{"line_number":34,"context_line":"orig_popen_wait \u003d subprocess.Popen.wait"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def wait_with_timeout(self, timeout\u003dONCE_TIMEOUT, check_interval\u003d0.01):"},{"line_number":38,"context_line":"    # Note that we override the default timeout of None; no probe test"},{"line_number":39,"context_line":"    # should need to wait on even minute-long running processes."},{"line_number":40,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"01c7528f_832c62fe","line":37,"range":{"start_line":37,"start_character":28,"end_line":37,"end_character":48},"updated":"2024-11-14 10:54:40.000000000","message":"``timeout`` is explicitly set to None by ``subprocess.call`` and ``subprocess.check_call`` so this didn\u0027t work for me when I inserted a deliberate sleep in the relinker","commit_id":"2cc56922edd07ff4169af29cff13a3acb2d1fd8f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c3a720191fd89b217f3be0395a6e786007df317","unresolved":true,"context_lines":[{"line_number":47,"context_line":"        # happened."},{"line_number":48,"context_line":"        print(\u0027WARNING: killing long running daemon: %r\u0027 % self.args)"},{"line_number":49,"context_line":"        self.kill()"},{"line_number":50,"context_line":"        return self.returncode"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"subprocess.Popen.wait \u003d wait_with_timeout"}],"source_content_type":"text/x-python","patch_set":4,"id":"c135e876_176b26ba","line":50,"updated":"2024-11-14 10:54:40.000000000","message":"``self.returncode`` will be None which ``check_call`` morphs to ``0`` :( so even when I got the timeout to fire the relinker probe test still passed (albeit with a WARNING).\n\nIf we\u0027re confident that daemons shouldn\u0027t run for more than 30secs then we should fail the test if they do.","commit_id":"2cc56922edd07ff4169af29cff13a3acb2d1fd8f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"43829bdcfe8e0c1edea956981e506abaef953e1c","unresolved":true,"context_lines":[{"line_number":47,"context_line":"        # happened."},{"line_number":48,"context_line":"        print(\u0027WARNING: killing long running daemon: %r\u0027 % self.args)"},{"line_number":49,"context_line":"        self.kill()"},{"line_number":50,"context_line":"        return self.returncode"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"subprocess.Popen.wait \u003d wait_with_timeout"}],"source_content_type":"text/x-python","patch_set":4,"id":"e6906a46_b4eca5d3","line":50,"in_reply_to":"a890c265_7d53cf2d","updated":"2024-11-14 18:35:47.000000000","message":"\u003e I think we\u0027d just find ourselves frustrated by intermittent probe test failures.\n\nOn second thought, we already retry failed probe tests once -- maybe this will be enough to stop the overall failures (or at least, reduce them to the point that we hardly notice them).","commit_id":"2cc56922edd07ff4169af29cff13a3acb2d1fd8f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"707980fa7e61e9de288af7152bf0d33f4dafd4aa","unresolved":true,"context_lines":[{"line_number":47,"context_line":"        # happened."},{"line_number":48,"context_line":"        print(\u0027WARNING: killing long running daemon: %r\u0027 % self.args)"},{"line_number":49,"context_line":"        self.kill()"},{"line_number":50,"context_line":"        return self.returncode"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"subprocess.Popen.wait \u003d wait_with_timeout"}],"source_content_type":"text/x-python","patch_set":4,"id":"a890c265_7d53cf2d","line":50,"in_reply_to":"c135e876_176b26ba","updated":"2024-11-14 16:45:27.000000000","message":"I\u0027d say I\u0027m confident that they *shouldn\u0027t* run for more than 30s, but in light of https://github.com/eventlet/eventlet/issues/989 I\u0027m not confident that they *won\u0027t*. I\u0027m reasonably confident (I think) that if a subprocess is running more than 30s, it tripped that bug, and so it did all the work it wanted to do.\n\nI suppose a fast failure is better than a job timeout, but I think we\u0027d just find ourselves frustrated by intermittent probe test failures.\n\n\u003e so even when I got the timeout to fire the relinker probe test still passed (albeit with a WARNING).\n\nThat sounds like we haven\u0027t made sufficient assertions about state on disk following a relinker run. (Or the added sleep was at the end rather than the beginning.)","commit_id":"2cc56922edd07ff4169af29cff13a3acb2d1fd8f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"403383796da5916b4a68beb9465d448a3bd686ac","unresolved":true,"context_lines":[{"line_number":51,"context_line":"        self.kill()"},{"line_number":52,"context_line":"        # return 128 + 9 \u003d 137 which is same as if using a command line like"},{"line_number":53,"context_line":"        # \u0027timeout -s KILL \u003ctimeout\u003e \u003ccommand\u003e\u0027"},{"line_number":54,"context_line":"        return 137"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"subprocess.Popen.wait \u003d wait_with_timeout"}],"source_content_type":"text/x-python","patch_set":5,"id":"24533e7e_026389c0","line":54,"updated":"2024-11-14 11:00:00.000000000","message":"anything but zero would probably do but the relinker for example can return 0, 1 or 2 so I figured go for a big value that is clearly unexpected.","commit_id":"3ae9ed1915b7af692d67eb4aa33ed1f23fd16c94"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"60201b97bc318287043dfb04ff090d547cd8a96d","unresolved":true,"context_lines":[{"line_number":51,"context_line":"        self.kill()"},{"line_number":52,"context_line":"        # return 128 + 9 \u003d 137 which is same as if using a command line like"},{"line_number":53,"context_line":"        # \u0027timeout -s KILL \u003ctimeout\u003e \u003ccommand\u003e\u0027"},{"line_number":54,"context_line":"        return 137"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"subprocess.Popen.wait \u003d wait_with_timeout"}],"source_content_type":"text/x-python","patch_set":5,"id":"8a90f7f7_bb4dc2ec","line":54,"in_reply_to":"24533e7e_026389c0","updated":"2024-11-15 01:37:58.000000000","message":"FWIW, [the place I was most concerned with](https://github.com/openstack/swift/blob/2.34.0/swift/common/manager.py#L857) doesn\u0027t even check the return, so... sure?","commit_id":"3ae9ed1915b7af692d67eb4aa33ed1f23fd16c94"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e27b354af267d8b16957079ebdffcfe4bbf198c","unresolved":true,"context_lines":[{"line_number":51,"context_line":"        self.kill()"},{"line_number":52,"context_line":"        # return 128 + 9 \u003d 137 which is same as if using a command line like"},{"line_number":53,"context_line":"        # \u0027timeout -s KILL \u003ctimeout\u003e \u003ccommand\u003e\u0027"},{"line_number":54,"context_line":"        return 137"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"subprocess.Popen.wait \u003d wait_with_timeout"}],"source_content_type":"text/x-python","patch_set":5,"id":"8dfef9d2_44d874c0","line":54,"in_reply_to":"8a90f7f7_bb4dc2ec","updated":"2024-11-15 09:45:33.000000000","message":"but the change you made here does ;-) https://review.opendev.org/c/openstack/swift/+/934910/1/test/probe/test_object_partpower_increase.py","commit_id":"3ae9ed1915b7af692d67eb4aa33ed1f23fd16c94"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"403383796da5916b4a68beb9465d448a3bd686ac","unresolved":true,"context_lines":[{"line_number":54,"context_line":"        return 137"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"subprocess.Popen.wait \u003d wait_with_timeout"}],"source_content_type":"text/x-python","patch_set":5,"id":"63bc0de1_97654875","line":57,"updated":"2024-11-14 11:00:00.000000000","message":"Ideally I\u0027d prefer this to be more visible e.g. in the ``common.ProbeTest.setUp()`` and then unpatched in the ``tearDown``.","commit_id":"3ae9ed1915b7af692d67eb4aa33ed1f23fd16c94"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e27b354af267d8b16957079ebdffcfe4bbf198c","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        return 137"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"subprocess.Popen.wait \u003d wait_with_timeout"}],"source_content_type":"text/x-python","patch_set":5,"id":"167a2e6e_b921649b","line":57,"in_reply_to":"0054d139_21311b87","updated":"2024-11-15 09:45:33.000000000","message":"Done","commit_id":"3ae9ed1915b7af692d67eb4aa33ed1f23fd16c94"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"707980fa7e61e9de288af7152bf0d33f4dafd4aa","unresolved":true,"context_lines":[{"line_number":54,"context_line":"        return 137"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"subprocess.Popen.wait \u003d wait_with_timeout"}],"source_content_type":"text/x-python","patch_set":5,"id":"0054d139_21311b87","line":57,"in_reply_to":"63bc0de1_97654875","updated":"2024-11-14 16:45:27.000000000","message":"Seemed silly to have it flipping back and forth with every test, plus apparently [not all probe tests inherit from `ProbeTest`](https://github.com/openstack/swift/blob/2.34.0/test/probe/test_signals.py#L448) (!?)","commit_id":"3ae9ed1915b7af692d67eb4aa33ed1f23fd16c94"}]}