)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7b8b2c59818b996fd735448f368dffe2a5ad8bd3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6f9c655e_29508695","updated":"2025-05-28 14:29:09.000000000","message":"Thank you for inputs, will make the changes as per suggestions.","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"22959179bac85ad93cf8d7a65b8be8fa85a39c08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"fecfca43_711c687b","updated":"2025-05-29 13:37:34.000000000","message":"One bug in the test but otherwise looks great, thanks.","commit_id":"75c96949a09ed87e8b3b0604b56519cd7139de71"}],"glance/task_cancellation_tracker.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a3a04091c291298639fddea6732889a2022d4094","unresolved":true,"context_lines":[{"line_number":29,"context_line":"    if CONF.enabled_backends:"},{"line_number":30,"context_line":"        return CONF.os_glance_tasks_store.filesystem_store_datadir"},{"line_number":31,"context_line":"    else:"},{"line_number":32,"context_line":"        return CONF.node_staging_uri"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"def path_for_op(operation_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"62f24984_09a2d939","line":32,"updated":"2025-05-28 13:37:32.000000000","message":"These look good, but I wonder if we want to namespace the files or put them in another directory underneath? TBH, I don\u0027t even know what we store in the `os_glance_tasks_store` dir, but these files would be co-mingled with images in staging right?\n\nPerhaps just make `path_for_op()` prefix the `operation_id` with `running-task-` or something?","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7b8b2c59818b996fd735448f368dffe2a5ad8bd3","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    if CONF.enabled_backends:"},{"line_number":30,"context_line":"        return CONF.os_glance_tasks_store.filesystem_store_datadir"},{"line_number":31,"context_line":"    else:"},{"line_number":32,"context_line":"        return CONF.node_staging_uri"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"def path_for_op(operation_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"a881c326_a5fc40be","line":32,"in_reply_to":"62f24984_09a2d939","updated":"2025-05-28 14:29:09.000000000","message":"Will make prefix operation_id with running-task-","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a3a04091c291298639fddea6732889a2022d4094","unresolved":true,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        # Wait for the system to acknowledge the cancellation"},{"line_number":72,"context_line":"        for _ in range(60):"},{"line_number":73,"context_line":"            if is_canceled(operation_id):"},{"line_number":74,"context_line":"                return"},{"line_number":75,"context_line":"            time.sleep(0.5)"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"e7751900_d26cae56","line":73,"updated":"2025-05-28 13:37:32.000000000","message":"This was a bug in my code, but I mentioned it on IRC. This will just make it immediately return because this is checking for the condition that we just set. You need a different thing here, just to wait for it to be removed (so `os.exists()`).","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7b8b2c59818b996fd735448f368dffe2a5ad8bd3","unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        # Wait for the system to acknowledge the cancellation"},{"line_number":72,"context_line":"        for _ in range(60):"},{"line_number":73,"context_line":"            if is_canceled(operation_id):"},{"line_number":74,"context_line":"                return"},{"line_number":75,"context_line":"            time.sleep(0.5)"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ad73c844_77d81504","line":73,"in_reply_to":"e7751900_d26cae56","updated":"2025-05-28 14:29:09.000000000","message":"Acknowledged","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a3a04091c291298639fddea6732889a2022d4094","unresolved":true,"context_lines":[{"line_number":79,"context_line":"                                    f\"task {operation_id}\")"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"def signal_canceled(operation_id):"},{"line_number":83,"context_line":"    \"\"\"Remove the lock file to signal that the operation is canceled.\"\"\""},{"line_number":84,"context_line":"    with lockutils.external_lock(\u0027tasks\u0027):"},{"line_number":85,"context_line":"        operation_path \u003d path_for_op(operation_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bb4bf289_e221a15b","line":82,"updated":"2025-05-28 13:37:32.000000000","message":"This is also \"signal finished\" right? Because we need to do this regardless of if it was canceled or not.","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7b8b2c59818b996fd735448f368dffe2a5ad8bd3","unresolved":false,"context_lines":[{"line_number":79,"context_line":"                                    f\"task {operation_id}\")"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"def signal_canceled(operation_id):"},{"line_number":83,"context_line":"    \"\"\"Remove the lock file to signal that the operation is canceled.\"\"\""},{"line_number":84,"context_line":"    with lockutils.external_lock(\u0027tasks\u0027):"},{"line_number":85,"context_line":"        operation_path \u003d path_for_op(operation_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"c9e979f9_f5bb1a5c","line":82,"in_reply_to":"bb4bf289_e221a15b","updated":"2025-05-28 14:29:09.000000000","message":"Acknowledged","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a3a04091c291298639fddea6732889a2022d4094","unresolved":true,"context_lines":[{"line_number":88,"context_line":"        except FileNotFoundError:"},{"line_number":89,"context_line":"            # If the file doesn\u0027t exist, it\u0027s already canceled or"},{"line_number":90,"context_line":"            # never registered"},{"line_number":91,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"62d95e02_588c883c","line":91,"updated":"2025-05-28 13:37:32.000000000","message":"We might want to `LOG.warning()` here. This really shouldn\u0027t happen and might mean that someone removed something underneath us.","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7b8b2c59818b996fd735448f368dffe2a5ad8bd3","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        except FileNotFoundError:"},{"line_number":89,"context_line":"            # If the file doesn\u0027t exist, it\u0027s already canceled or"},{"line_number":90,"context_line":"            # never registered"},{"line_number":91,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"5c095a5d_21589b4d","line":91,"in_reply_to":"62d95e02_588c883c","updated":"2025-05-28 14:29:09.000000000","message":"Acknowledged","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2e559009aeb1563887adbe51761ca7d4f2bfe435","unresolved":true,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"def cancel_operation(operation_id):"},{"line_number":68,"context_line":"    \"\"\"Mark an operation as canceled by creating a lock file.\"\"\""},{"line_number":69,"context_line":"    with lockutils.external_lock(\u0027tasks\u0027):"},{"line_number":70,"context_line":"        operation_path \u003d path_for_op(operation_id)"},{"line_number":71,"context_line":"        with open(operation_path, \u0027w\u0027) as f:"},{"line_number":72,"context_line":"            f.write(str(operation_id))"}],"source_content_type":"text/x-python","patch_set":5,"id":"f0ed2fec_e5a46791","line":69,"updated":"2025-05-29 14:57:23.000000000","message":"This needs to check for exists before we write, so that we won\u0027t \"register\" an operation by trying to cancel it. If the op isn\u0027t registered, then we should not wait.","commit_id":"dc2af9bd9d45877fdeaae452e01fa68e52ffd31f"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4dbef5bec3d164eb0219356b31937e3e1a9f6b4f","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"def cancel_operation(operation_id):"},{"line_number":68,"context_line":"    \"\"\"Mark an operation as canceled by creating a lock file.\"\"\""},{"line_number":69,"context_line":"    with lockutils.external_lock(\u0027tasks\u0027):"},{"line_number":70,"context_line":"        operation_path \u003d path_for_op(operation_id)"},{"line_number":71,"context_line":"        with open(operation_path, \u0027w\u0027) as f:"},{"line_number":72,"context_line":"            f.write(str(operation_id))"}],"source_content_type":"text/x-python","patch_set":5,"id":"690af222_ee36370c","line":69,"in_reply_to":"f0ed2fec_e5a46791","updated":"2025-05-29 17:08:05.000000000","message":"Done","commit_id":"dc2af9bd9d45877fdeaae452e01fa68e52ffd31f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8a7f6ccd27abf768c5f92130150570a069dc8024","unresolved":true,"context_lines":[{"line_number":70,"context_line":"    \"\"\""},{"line_number":71,"context_line":"    with lockutils.external_lock(\u0027tasks\u0027):"},{"line_number":72,"context_line":"        operation_path \u003d path_for_op(operation_id)"},{"line_number":73,"context_line":"        if os.path.exists(operation_path):"},{"line_number":74,"context_line":"            with open(operation_path, \u0027w\u0027) as f:"},{"line_number":75,"context_line":"                f.write(str(operation_id))"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"a6d6f0c5_1fdc7488","line":73,"updated":"2025-06-09 13:21:03.000000000","message":"If this is False, we will not cancel and then just proceed to the wait loop for no reason and then claim that it timed out, which is not right. I think you need to check this and raise if it doesn\u0027t exist, with an appropriate error.","commit_id":"26ec8b0d013a87824f8975414a4f4e121b0068ee"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d0cdb5353119ceae795df6cd4cdd5c1f37e174e8","unresolved":false,"context_lines":[{"line_number":70,"context_line":"    \"\"\""},{"line_number":71,"context_line":"    with lockutils.external_lock(\u0027tasks\u0027):"},{"line_number":72,"context_line":"        operation_path \u003d path_for_op(operation_id)"},{"line_number":73,"context_line":"        if os.path.exists(operation_path):"},{"line_number":74,"context_line":"            with open(operation_path, \u0027w\u0027) as f:"},{"line_number":75,"context_line":"                f.write(str(operation_id))"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1bfc8595_6f94ce43","line":73,"in_reply_to":"a6d6f0c5_1fdc7488","updated":"2025-06-09 13:45:33.000000000","message":"Acknowledged","commit_id":"26ec8b0d013a87824f8975414a4f4e121b0068ee"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"0933b8395bae2e376169abba0eea2e5d5138d605","unresolved":true,"context_lines":[{"line_number":90,"context_line":""},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"def signal_finished(operation_id):"},{"line_number":93,"context_line":"    \"\"\"Remove the lock file to signal that the operation is canceled.\"\"\""},{"line_number":94,"context_line":"    with lockutils.external_lock(\u0027tasks\u0027):"},{"line_number":95,"context_line":"        operation_path \u003d path_for_op(operation_id)"},{"line_number":96,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"47200268_05c12557","line":93,"range":{"start_line":93,"start_character":23,"end_line":93,"end_character":27},"updated":"2025-06-13 18:23:47.000000000","message":"So \"signal_finished\" is called whether the operation has been cancelled or has been completed normally, right? If so, I suggest updating the docstring.","commit_id":"1311d5fe2f1e308e1abd930ab17a8fb368b670fe"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"9d59a1d16c2bcbe02bc49f0da1f95a97ed49897c","unresolved":true,"context_lines":[{"line_number":90,"context_line":""},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"def signal_finished(operation_id):"},{"line_number":93,"context_line":"    \"\"\"Remove the lock file to signal that the operation is canceled.\"\"\""},{"line_number":94,"context_line":"    with lockutils.external_lock(\u0027tasks\u0027):"},{"line_number":95,"context_line":"        operation_path \u003d path_for_op(operation_id)"},{"line_number":96,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"aec2f826_f9b121f8","line":93,"range":{"start_line":93,"start_character":23,"end_line":93,"end_character":27},"in_reply_to":"45ba3d3d_f321ced7","updated":"2025-06-13 21:20:53.000000000","message":"Oh, ok, I had no idea :D Thanks for explaining.","commit_id":"1311d5fe2f1e308e1abd930ab17a8fb368b670fe"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fe1a819bd07a1e517af355d44b6502a7e18d8d05","unresolved":true,"context_lines":[{"line_number":90,"context_line":""},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"def signal_finished(operation_id):"},{"line_number":93,"context_line":"    \"\"\"Remove the lock file to signal that the operation is canceled.\"\"\""},{"line_number":94,"context_line":"    with lockutils.external_lock(\u0027tasks\u0027):"},{"line_number":95,"context_line":"        operation_path \u003d path_for_op(operation_id)"},{"line_number":96,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"45ba3d3d_f321ced7","line":93,"range":{"start_line":93,"start_character":23,"end_line":93,"end_character":27},"in_reply_to":"47200268_05c12557","updated":"2025-06-13 19:53:32.000000000","message":"Sure, that\u0027s fair. In some realms, \"canceled\" means \"finished\". Like in (US at least) you received \"canceled checks\" back after they have been deposited successfully. Sort of a \"this has served its purpose and now it is being \u0027canceled\u0027 so it doesn\u0027t get used again\" type thing.","commit_id":"1311d5fe2f1e308e1abd930ab17a8fb368b670fe"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"987f12eb194dcbe35f2f1680339e0efc26531ccc","unresolved":false,"context_lines":[{"line_number":90,"context_line":""},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"def signal_finished(operation_id):"},{"line_number":93,"context_line":"    \"\"\"Remove the lock file to signal that the operation is canceled.\"\"\""},{"line_number":94,"context_line":"    with lockutils.external_lock(\u0027tasks\u0027):"},{"line_number":95,"context_line":"        operation_path \u003d path_for_op(operation_id)"},{"line_number":96,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a1cef8b_4fc783c4","line":93,"range":{"start_line":93,"start_character":23,"end_line":93,"end_character":27},"in_reply_to":"aec2f826_f9b121f8","updated":"2025-06-16 06:24:58.000000000","message":"Done","commit_id":"1311d5fe2f1e308e1abd930ab17a8fb368b670fe"}],"glance/tests/unit/test_task_cancellation_tracker.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a3a04091c291298639fddea6732889a2022d4094","unresolved":true,"context_lines":[{"line_number":48,"context_line":"        self.assertTrue(tracker.is_canceled(op_id))"},{"line_number":49,"context_line":"        # File should exist and be nonzero"},{"line_number":50,"context_line":"        path \u003d tracker.path_for_op(op_id)"},{"line_number":51,"context_line":"        self.assertTrue(os.path.getsize(path) \u003e 0)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def test_signal_canceled(self):"},{"line_number":54,"context_line":"        op_id \u003d \u0027op3\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"286c69a6_219d19fa","line":51,"updated":"2025-05-28 13:37:32.000000000","message":"`assertGreater()`","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7b8b2c59818b996fd735448f368dffe2a5ad8bd3","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self.assertTrue(tracker.is_canceled(op_id))"},{"line_number":49,"context_line":"        # File should exist and be nonzero"},{"line_number":50,"context_line":"        path \u003d tracker.path_for_op(op_id)"},{"line_number":51,"context_line":"        self.assertTrue(os.path.getsize(path) \u003e 0)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def test_signal_canceled(self):"},{"line_number":54,"context_line":"        op_id \u003d \u0027op3\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"d6aa038e_a328932d","line":51,"in_reply_to":"286c69a6_219d19fa","updated":"2025-05-28 14:29:09.000000000","message":"Acknowledged","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a3a04091c291298639fddea6732889a2022d4094","unresolved":true,"context_lines":[{"line_number":49,"context_line":"        # File should exist and be nonzero"},{"line_number":50,"context_line":"        path \u003d tracker.path_for_op(op_id)"},{"line_number":51,"context_line":"        self.assertTrue(os.path.getsize(path) \u003e 0)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def test_signal_canceled(self):"},{"line_number":54,"context_line":"        op_id \u003d \u0027op3\u0027"},{"line_number":55,"context_line":"        tracker.register_operation(op_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"d8e63bfb_1ba9e427","line":52,"updated":"2025-05-28 13:37:32.000000000","message":"You\u0027re missing a test for the happy path where we wait for a bit and then find it canceled. Easy way to do that is to patch `time.sleep()` with a `side_effect` function that does nothing for the first three calls, then does the cancel for us.","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7b8b2c59818b996fd735448f368dffe2a5ad8bd3","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        # File should exist and be nonzero"},{"line_number":50,"context_line":"        path \u003d tracker.path_for_op(op_id)"},{"line_number":51,"context_line":"        self.assertTrue(os.path.getsize(path) \u003e 0)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def test_signal_canceled(self):"},{"line_number":54,"context_line":"        op_id \u003d \u0027op3\u0027"},{"line_number":55,"context_line":"        tracker.register_operation(op_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f2176eeb_c60930c3","line":52,"in_reply_to":"d8e63bfb_1ba9e427","updated":"2025-05-28 14:29:09.000000000","message":"Acknowledged","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a3a04091c291298639fddea6732889a2022d4094","unresolved":true,"context_lines":[{"line_number":73,"context_line":"        with mock.patch.object(tracker, \u0027is_canceled\u0027,"},{"line_number":74,"context_line":"                               return_value\u003dFalse):"},{"line_number":75,"context_line":"            self.assertRaises("},{"line_number":76,"context_line":"                exception.ServerError, tracker.cancel_operation, op_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"b60c7e23_5fdd2646","line":76,"updated":"2025-05-28 13:37:32.000000000","message":"This will eat 60s from the test run, right? I think you can patch `time.sleep` to be a no-op and then also check that it was called.","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7b8b2c59818b996fd735448f368dffe2a5ad8bd3","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        with mock.patch.object(tracker, \u0027is_canceled\u0027,"},{"line_number":74,"context_line":"                               return_value\u003dFalse):"},{"line_number":75,"context_line":"            self.assertRaises("},{"line_number":76,"context_line":"                exception.ServerError, tracker.cancel_operation, op_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"6a7291e8_6940f0fe","line":76,"in_reply_to":"b60c7e23_5fdd2646","updated":"2025-05-28 14:29:09.000000000","message":"it waits for 30 seconds, but I will modify the test as suggested!!","commit_id":"ab71623087941c03359cc03ec06db6f4993c3154"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"22959179bac85ad93cf8d7a65b8be8fa85a39c08","unresolved":true,"context_lines":[{"line_number":86,"context_line":"                return False"},{"line_number":87,"context_line":"            return True"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"        with mock.patch(\u0027os.path.exists\u0027, side_effect\u003dside_effect):"},{"line_number":90,"context_line":"            with mock.patch(\u0027os.path.getsize\u0027, return_value\u003d1):"},{"line_number":91,"context_line":"                tracker.cancel_operation(op_id)"},{"line_number":92,"context_line":"        self.assertGreaterEqual(mock_sleep.call_count, 3)"}],"source_content_type":"text/x-python","patch_set":4,"id":"cbcca70c_d268fde4","line":89,"updated":"2025-05-29 13:37:34.000000000","message":"Note that you can do `side_effect \u003d [True, True, False]` and achieve the same thing without the function. For the function, I was thinking we *actually* remove the file so we\u0027re not mocking exists, but this works too, no need to change it.","commit_id":"75c96949a09ed87e8b3b0604b56519cd7139de71"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"40f30c21c6ffba2821d581a683cc6b2b4208b68f","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                return False"},{"line_number":87,"context_line":"            return True"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"        with mock.patch(\u0027os.path.exists\u0027, side_effect\u003dside_effect):"},{"line_number":90,"context_line":"            with mock.patch(\u0027os.path.getsize\u0027, return_value\u003d1):"},{"line_number":91,"context_line":"                tracker.cancel_operation(op_id)"},{"line_number":92,"context_line":"        self.assertGreaterEqual(mock_sleep.call_count, 3)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1184979e_623ff841","line":89,"in_reply_to":"cbcca70c_d268fde4","updated":"2025-05-29 14:44:30.000000000","message":"Acknowledged","commit_id":"75c96949a09ed87e8b3b0604b56519cd7139de71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"22959179bac85ad93cf8d7a65b8be8fa85a39c08","unresolved":true,"context_lines":[{"line_number":89,"context_line":"        with mock.patch(\u0027os.path.exists\u0027, side_effect\u003dside_effect):"},{"line_number":90,"context_line":"            with mock.patch(\u0027os.path.getsize\u0027, return_value\u003d1):"},{"line_number":91,"context_line":"                tracker.cancel_operation(op_id)"},{"line_number":92,"context_line":"        self.assertGreaterEqual(mock_sleep.call_count, 3)"}],"source_content_type":"text/x-python","patch_set":4,"id":"44bdee3d_fbc5dd11","line":92,"updated":"2025-05-29 13:37:34.000000000","message":"This should be `assertEqual()`. If it\u0027s called more than three times then something is wrong. This would also pass if we don\u0027t cancel and try all 60 times.","commit_id":"75c96949a09ed87e8b3b0604b56519cd7139de71"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"40f30c21c6ffba2821d581a683cc6b2b4208b68f","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        with mock.patch(\u0027os.path.exists\u0027, side_effect\u003dside_effect):"},{"line_number":90,"context_line":"            with mock.patch(\u0027os.path.getsize\u0027, return_value\u003d1):"},{"line_number":91,"context_line":"                tracker.cancel_operation(op_id)"},{"line_number":92,"context_line":"        self.assertGreaterEqual(mock_sleep.call_count, 3)"}],"source_content_type":"text/x-python","patch_set":4,"id":"f0625155_66e4cb5a","line":92,"in_reply_to":"44bdee3d_fbc5dd11","updated":"2025-05-29 14:44:30.000000000","message":"Done","commit_id":"75c96949a09ed87e8b3b0604b56519cd7139de71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8a7f6ccd27abf768c5f92130150570a069dc8024","unresolved":true,"context_lines":[{"line_number":71,"context_line":"    @mock.patch(\u0027os.path.exists\u0027)"},{"line_number":72,"context_line":"    @mock.patch(\u0027time.sleep\u0027, return_value\u003dNone)"},{"line_number":73,"context_line":"    def test_cancel_operation_timeout(self, mock_sleep, mock_exists):"},{"line_number":74,"context_line":"        mock_exists.return_value \u003d True"},{"line_number":75,"context_line":"        op_id \u003d \u0027op6\u0027"},{"line_number":76,"context_line":"        self.assertRaises("},{"line_number":77,"context_line":"            exception.ServerError, tracker.cancel_operation, op_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fac3449_211faa44","line":74,"updated":"2025-06-09 13:21:03.000000000","message":"Need a test case for False, ensuring the right error is raised and sleep is never called.","commit_id":"26ec8b0d013a87824f8975414a4f4e121b0068ee"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d0cdb5353119ceae795df6cd4cdd5c1f37e174e8","unresolved":true,"context_lines":[{"line_number":71,"context_line":"    @mock.patch(\u0027os.path.exists\u0027)"},{"line_number":72,"context_line":"    @mock.patch(\u0027time.sleep\u0027, return_value\u003dNone)"},{"line_number":73,"context_line":"    def test_cancel_operation_timeout(self, mock_sleep, mock_exists):"},{"line_number":74,"context_line":"        mock_exists.return_value \u003d True"},{"line_number":75,"context_line":"        op_id \u003d \u0027op6\u0027"},{"line_number":76,"context_line":"        self.assertRaises("},{"line_number":77,"context_line":"            exception.ServerError, tracker.cancel_operation, op_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f971c74_4a7d061b","line":74,"in_reply_to":"1fac3449_211faa44","updated":"2025-06-09 13:45:33.000000000","message":"Test at line 53 is enough?","commit_id":"26ec8b0d013a87824f8975414a4f4e121b0068ee"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ebe5cff01e9952d26da0109a7d2c776cf9ce970c","unresolved":true,"context_lines":[{"line_number":71,"context_line":"    @mock.patch(\u0027os.path.exists\u0027)"},{"line_number":72,"context_line":"    @mock.patch(\u0027time.sleep\u0027, return_value\u003dNone)"},{"line_number":73,"context_line":"    def test_cancel_operation_timeout(self, mock_sleep, mock_exists):"},{"line_number":74,"context_line":"        mock_exists.return_value \u003d True"},{"line_number":75,"context_line":"        op_id \u003d \u0027op6\u0027"},{"line_number":76,"context_line":"        self.assertRaises("},{"line_number":77,"context_line":"            exception.ServerError, tracker.cancel_operation, op_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"c4e25b70_9f629fa5","line":74,"in_reply_to":"3f971c74_4a7d061b","updated":"2025-06-09 14:12:42.000000000","message":"No, because that test isn\u0027t testing the case where we try to cancel something that hasn\u0027t been started and a non-timeout error is raised. Maybe after you fix the cancel behavior that one will be changed into the right test, I\u0027m not sure, since it\u0027s clearly registering the operation *and* reporting that the file doesn\u0027t exist. Seems like that test is looking to test the cancel wait and not the case of canceling something not registered right?","commit_id":"26ec8b0d013a87824f8975414a4f4e121b0068ee"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"5d4a56253e20d884977142cf745e7097d17e6003","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    @mock.patch(\u0027os.path.exists\u0027)"},{"line_number":72,"context_line":"    @mock.patch(\u0027time.sleep\u0027, return_value\u003dNone)"},{"line_number":73,"context_line":"    def test_cancel_operation_timeout(self, mock_sleep, mock_exists):"},{"line_number":74,"context_line":"        mock_exists.return_value \u003d True"},{"line_number":75,"context_line":"        op_id \u003d \u0027op6\u0027"},{"line_number":76,"context_line":"        self.assertRaises("},{"line_number":77,"context_line":"            exception.ServerError, tracker.cancel_operation, op_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"0cb24204_02c61c0e","line":74,"in_reply_to":"c4e25b70_9f629fa5","updated":"2025-06-09 15:55:20.000000000","message":"Acknowledged","commit_id":"26ec8b0d013a87824f8975414a4f4e121b0068ee"}]}