)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":35759,"name":"Yian Zong","display_name":"Yian Zong","email":"yian.zong@dell.com","username":"yianzong"},"change_message_id":"7d80d5f520fdfb129220b9a19382c8a5b7c24ec1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c20e48e4_461ec055","updated":"2024-07-12 03:10:34.000000000","message":"Code changes and UT LGTM.","commit_id":"44b2af4fff658a5760c9d941d923d6e6b44d275a"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"01021c910c5d53ba5c1a299181e9c29c5676a3df","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8fc03eba_69bc0a52","updated":"2024-08-21 15:10:22.000000000","message":"Great catch!!!\nThe downvote is because I think we should preserve existing behavior.","commit_id":"44b2af4fff658a5760c9d941d923d6e6b44d275a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"85ba26c12c5ae1f7a6b08a59c371a155124e0394","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9326dc74_aa680b0f","updated":"2024-08-21 14:44:00.000000000","message":"I feel this requires some discussion around what is the expected behavior for raise_timeout when check_exit_code\u003d0\nAlso can you report a bug stating where you are facing this issue?","commit_id":"44b2af4fff658a5760c9d941d923d6e6b44d275a"},{"author":{"_account_id":29074,"name":"Felix Huettner","email":"felix.huettner@digits.schwarz","username":"felix.huettner"},"change_message_id":"5af7b5862d73eceef94368192442e3c04a5a0067","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a9a76804_2ba9ad62","updated":"2024-06-27 14:48:37.000000000","message":"recheck","commit_id":"44b2af4fff658a5760c9d941d923d6e6b44d275a"},{"author":{"_account_id":29074,"name":"Felix Huettner","email":"felix.huettner@digits.schwarz","username":"felix.huettner"},"change_message_id":"785268b9eee44b08b96564f89b3db199117e633c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"65802928_e926aaa7","in_reply_to":"9326dc74_aa680b0f","updated":"2024-08-27 08:51:43.000000000","message":"The issue we had was that was that cinder-backup was mounting an NFS share and it timed out after 10 min. However cinder-backup then continued to write to the non-mounted directory as it never got any kind of error.\n\nregarding raise_timeout a quick grep showed that it is never actually set to False (just in some testcase). So we can assume raise_timeout to always be True in practice.\n\nEspecially the remotefs code is using check_exit_code\u003d0 a lot for the mount and umount commands. In these cases i would expect an exception both when the exit code does not match and when the timeout is hit, otherwise we might use a non-mounted directory for storing things.","commit_id":"44b2af4fff658a5760c9d941d923d6e6b44d275a"}],"os_brick/privileged/rootwrap.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"01021c910c5d53ba5c1a299181e9c29c5676a3df","unresolved":true,"context_lines":[{"line_number":135,"context_line":"    # set the more than reasonable timeout of 10 minutes (600 seconds)."},{"line_number":136,"context_line":"    timeout \u003d kwargs.pop(\u0027timeout\u0027, 600)"},{"line_number":137,"context_line":"    sig_end \u003d kwargs.pop(\u0027signal\u0027, signal.SIGTERM)"},{"line_number":138,"context_line":"    raise_timeout \u003d kwargs.pop(\u0027raise_timeout\u0027, True)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    on_execute_call \u003d kwargs.pop(\u0027on_execute\u0027, None)"},{"line_number":141,"context_line":"    on_completion_call \u003d kwargs.pop(\u0027on_completion\u0027, None)"}],"source_content_type":"text/x-python","patch_set":2,"id":"58bfb595_4874689b","line":138,"updated":"2024-08-21 15:10:22.000000000","message":"-1: I think we should keep the behavior described in L82-L83.\n\n```\n   default_raise_timeout \u003d kwargs.get(\u0027check_exit_code\u0027, True)\n   raise_timeout \u003d kwargs.pop(\u0027raise_timeout\u0027, default_raise_timeout is not False)\n```","commit_id":"44b2af4fff658a5760c9d941d923d6e6b44d275a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"85ba26c12c5ae1f7a6b08a59c371a155124e0394","unresolved":true,"context_lines":[{"line_number":135,"context_line":"    # set the more than reasonable timeout of 10 minutes (600 seconds)."},{"line_number":136,"context_line":"    timeout \u003d kwargs.pop(\u0027timeout\u0027, 600)"},{"line_number":137,"context_line":"    sig_end \u003d kwargs.pop(\u0027signal\u0027, signal.SIGTERM)"},{"line_number":138,"context_line":"    raise_timeout \u003d kwargs.pop(\u0027raise_timeout\u0027, True)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    on_execute_call \u003d kwargs.pop(\u0027on_execute\u0027, None)"},{"line_number":141,"context_line":"    on_completion_call \u003d kwargs.pop(\u0027on_completion\u0027, None)"}],"source_content_type":"text/x-python","patch_set":2,"id":"6e424cc2_ac609744","line":138,"range":{"start_line":138,"start_character":4,"end_line":138,"end_character":53},"updated":"2024-08-21 14:44:00.000000000","message":"Currently we are ignoring the check_exit_code variable altogether for the default of raise_timeout so it would be better to understand the original intent of introducing it as a default for raise_timeout\n\nIf all values of check_exit_code was expected to be true, then the default introduced was clearly a bug but if the the intent was to only treat non-zero values as true then the logic is working correctly and should not require this change.","commit_id":"44b2af4fff658a5760c9d941d923d6e6b44d275a"},{"author":{"_account_id":29074,"name":"Felix Huettner","email":"felix.huettner@digits.schwarz","username":"felix.huettner"},"change_message_id":"785268b9eee44b08b96564f89b3db199117e633c","unresolved":true,"context_lines":[{"line_number":135,"context_line":"    # set the more than reasonable timeout of 10 minutes (600 seconds)."},{"line_number":136,"context_line":"    timeout \u003d kwargs.pop(\u0027timeout\u0027, 600)"},{"line_number":137,"context_line":"    sig_end \u003d kwargs.pop(\u0027signal\u0027, signal.SIGTERM)"},{"line_number":138,"context_line":"    raise_timeout \u003d kwargs.pop(\u0027raise_timeout\u0027, True)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    on_execute_call \u003d kwargs.pop(\u0027on_execute\u0027, None)"},{"line_number":141,"context_line":"    on_completion_call \u003d kwargs.pop(\u0027on_completion\u0027, None)"}],"source_content_type":"text/x-python","patch_set":2,"id":"eaf719f2_e9c74e02","line":138,"in_reply_to":"58bfb595_4874689b","updated":"2024-08-27 08:51:43.000000000","message":"sounds good, adapted it","commit_id":"44b2af4fff658a5760c9d941d923d6e6b44d275a"},{"author":{"_account_id":29074,"name":"Felix Huettner","email":"felix.huettner@digits.schwarz","username":"felix.huettner"},"change_message_id":"785268b9eee44b08b96564f89b3db199117e633c","unresolved":true,"context_lines":[{"line_number":135,"context_line":"    # set the more than reasonable timeout of 10 minutes (600 seconds)."},{"line_number":136,"context_line":"    timeout \u003d kwargs.pop(\u0027timeout\u0027, 600)"},{"line_number":137,"context_line":"    sig_end \u003d kwargs.pop(\u0027signal\u0027, signal.SIGTERM)"},{"line_number":138,"context_line":"    raise_timeout \u003d kwargs.pop(\u0027raise_timeout\u0027, True)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    on_execute_call \u003d kwargs.pop(\u0027on_execute\u0027, None)"},{"line_number":141,"context_line":"    on_completion_call \u003d kwargs.pop(\u0027on_completion\u0027, None)"}],"source_content_type":"text/x-python","patch_set":2,"id":"4b1e686a_4630985e","line":138,"range":{"start_line":138,"start_character":4,"end_line":138,"end_character":53},"in_reply_to":"6e424cc2_ac609744","updated":"2024-08-27 08:51:43.000000000","message":"The issue here is that this whole function was introduced in https://github.com/openstack/os-brick/commit/400ca5d6db818b966065001571e59198c6966e2f#diff-f3deb1d16798da7c5dbd078134f0e7f229eb2b89026fe06160b5034afce9ef78 which is just a gigantic change. The commit message also does not mention anything about that.\n\nHowever i would assume that this goes back to when check_exit_code was only used with booleans, then this issue does not actually appear.","commit_id":"44b2af4fff658a5760c9d941d923d6e6b44d275a"}]}