)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7bd51546d914351b67a7cac33e5009c156623463","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Abhishek Kekane \u003cakekane@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2025-06-05 14:55:15 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Cancel hasing operation if image is deleted"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Used file based locking to track the progress of hash calculation"},{"line_number":10,"context_line":"task. If image is deleted in between and delete call is not"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"f6be6978_341afeda","line":7,"range":{"start_line":7,"start_character":7,"end_line":7,"end_character":13},"updated":"2025-06-09 13:28:16.000000000","message":"\"hashing\"","commit_id":"4d253fed2dc25dbf8c00d364e513b761bf2fc6c9"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7e4d0731ae4ab2b7b6372cbe27db0caa8ac878d1","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Abhishek Kekane \u003cakekane@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2025-06-05 14:55:15 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Cancel hasing operation if image is deleted"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Used file based locking to track the progress of hash calculation"},{"line_number":10,"context_line":"task. If image is deleted in between and delete call is not"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"c10532e2_6f7897c0","line":7,"range":{"start_line":7,"start_character":7,"end_line":7,"end_character":13},"in_reply_to":"f5b1a148_01f8f68c","updated":"2025-06-11 15:07:45.000000000","message":"Done","commit_id":"4d253fed2dc25dbf8c00d364e513b761bf2fc6c9"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"833fdc3ee6d6d7c615cb35c057e6fa510a07ea61","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Abhishek Kekane \u003cakekane@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2025-06-05 14:55:15 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Cancel hasing operation if image is deleted"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Used file based locking to track the progress of hash calculation"},{"line_number":10,"context_line":"task. If image is deleted in between and delete call is not"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"f5b1a148_01f8f68c","line":7,"range":{"start_line":7,"start_character":7,"end_line":7,"end_character":13},"in_reply_to":"f6be6978_341afeda","updated":"2025-06-09 16:38:11.000000000","message":"will fix after current CI run 😞","commit_id":"4d253fed2dc25dbf8c00d364e513b761bf2fc6c9"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"76ca06597cfaab8f3ba0f8384a86ce771326f185","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f0651960_9321c6c4","updated":"2025-05-26 15:30:34.000000000","message":"Impressive!","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a3f8d1738e32c6ad3918e5df4d4a6bf183c0ec28","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c84cb602_3b85fc3c","updated":"2025-05-26 17:52:23.000000000","message":"Working on fixing tests, New PS coming may be in couple of hours","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d8ccb50b314b21facec39b9f8b08a6a912d554b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f45165a3_8f7c54f7","updated":"2025-05-27 14:45:03.000000000","message":"I think we should avoid bringing memcache into this if we can help it. Also, the delete proxy should be synchronous and complete, like the import example already does. Meaning we should not \"signal and wait 5s then complete the delete ourselves\" but rather delegate the delete itself to the process we\u0027re proxying to, avoiding the need to try to do this rendezvous and any sort of two-way signaling to make it reliable. Just asking the owner of the task to both cancel *and* do the delete means you can do all the synchronization in one place and avoid the external dependencies, races, etc. Also, all the logs for everything will be on a single worker, so much easier to debug. The worker that receives the request will be doing nothing but the proxy, but the target host will do the task itself, the cancel, the wait, etc.","commit_id":"8d2ec7a35b063057c0bb82a264cf62d98b426757"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"1ae6031f22dc1b60701ed7926835e708f2f4f971","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"75cffac7_a5ec08e3","updated":"2025-05-27 20:25:29.000000000","message":"This should be rebased on top of https://review.opendev.org/c/openstack/glance/+/951031","commit_id":"8d2ec7a35b063057c0bb82a264cf62d98b426757"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"842058c2d5d4b1dca0809604a82cba54c26768ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"d8c74eef_63ba68f4","updated":"2025-06-04 16:27:04.000000000","message":"We can see tempest test passing on top of this patch\n\nJun 04 15:51:20.610285 np0041022461 devstack@g-api.service[88526]: DEBUG glance.async_.flows.location_import [-] Hash calculation cancelled: Hash calculation for image 22a511fe-865f-4015-97d3-1733cd0021bc has been canceled {{(pid\u003d88526) _set_checksum_and_hash /opt/stack/glance/glance/async_/flows/location_import.py:91}}","commit_id":"b664301bc750aa6dcdfa58d6fdffac91ffff2957"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7bd51546d914351b67a7cac33e5009c156623463","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"2cb85677_31b46d13","updated":"2025-06-09 13:28:16.000000000","message":"I don\u0027t see where (in the above .zuul patch) the new tempest test is actually running to verify this. Can you point me to it?","commit_id":"4d253fed2dc25dbf8c00d364e513b761bf2fc6c9"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3d615dc352aa1e9d2dc58f88935e49d01c432fcd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"2f9b1c21_d9d5f205","updated":"2025-06-05 17:38:55.000000000","message":"recheck","commit_id":"4d253fed2dc25dbf8c00d364e513b761bf2fc6c9"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"18e46a437b95814fb35b4d579a2b4978f0293115","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"36ea64cb_447182d3","in_reply_to":"2cb85677_31b46d13","updated":"2025-06-09 13:36:48.000000000","message":"https://review.opendev.org/c/openstack/glance/+/951596/3\n\nI need to rebase another latest patch on top of the cancel hash operation patch","commit_id":"4d253fed2dc25dbf8c00d364e513b761bf2fc6c9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d52f5c612eeca853eb8f64ae6dc3caa2cc65ef9f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"0320a729_6536bd9c","updated":"2025-06-10 16:55:27.000000000","message":"Reminder to fix the typo in the subject :)","commit_id":"0057236a239c91480b1efa76108a0419548371da"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"486fd9f14f2e6c903a094fd522079e149b14c525","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"8cb6a6ef_4f215838","updated":"2025-06-10 14:08:14.000000000","message":"The test does seem to be poking the cancel button from the remote and the hashing is successfully canceled, so that\u0027s good. Just a concern inline about the resulting behavior of the cancel.","commit_id":"0057236a239c91480b1efa76108a0419548371da"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7e4d0731ae4ab2b7b6372cbe27db0caa8ac878d1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"10db9ae0_b48f245d","in_reply_to":"0320a729_6536bd9c","updated":"2025-06-11 15:07:45.000000000","message":"Done","commit_id":"0057236a239c91480b1efa76108a0419548371da"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5ef982b5b9d6114974433bc9cbfb1ebb9dad2d83","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"bff86a90_257a9800","updated":"2025-06-13 14:47:40.000000000","message":"Looks good and the test report shows the task being reverted now. One comment inline about bleeding the details out of the tracker. If we moved the tracker to use some other mechanism to do all the signaling, this code would silently stop doing what we expect.","commit_id":"7081cc9592ccef209e753bd2dcadc79b144c7463"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f795c7fa6a333f182bfcd17226a2cdcf260ef0cd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9d86c30c_36935c95","updated":"2025-06-12 08:08:05.000000000","message":"recheck grenade time out","commit_id":"7081cc9592ccef209e753bd2dcadc79b144c7463"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"52c7309f30016855f955c4dea92c9459aff19b17","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"81338639_bf380f75","updated":"2025-06-12 13:39:01.000000000","message":"recheck server creation failure","commit_id":"7081cc9592ccef209e753bd2dcadc79b144c7463"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"67db30f560f4e259ef02df1ab6ca54cac610d598","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"f118424f_172a6efa","updated":"2025-06-16 14:12:33.000000000","message":"Thanks for the added explanation!","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"}],"glance/api/v2/images.py":[{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"76ca06597cfaab8f3ba0f8384a86ce771326f185","unresolved":true,"context_lines":[{"line_number":238,"context_line":"                      {\u0027image\u0027: image.image_id, \u0027task\u0027: task.task_id,"},{"line_number":239,"context_line":"                       \u0027keys\u0027: \u0027,\u0027.join(changed)})"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"    def _proxy_request(self, image, req, host\u003d\u0027os_glance_stage_host\u0027,"},{"line_number":242,"context_line":"                       body\u003dNone):"},{"line_number":243,"context_line":"        \"\"\"Proxy a request to a staging host."},{"line_number":244,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"0e092570_82166bb6","line":241,"range":{"start_line":241,"start_character":45,"end_line":241,"end_character":46},"updated":"2025-05-26 15:30:34.000000000","message":"I find it weird to have a default value for \"host\" here. There are not many calls to _proxy_request, so I think we should manually edit them all and have \"host\" be a required parameter.","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a3f8d1738e32c6ad3918e5df4d4a6bf183c0ec28","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                      {\u0027image\u0027: image.image_id, \u0027task\u0027: task.task_id,"},{"line_number":239,"context_line":"                       \u0027keys\u0027: \u0027,\u0027.join(changed)})"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"    def _proxy_request(self, image, req, host\u003d\u0027os_glance_stage_host\u0027,"},{"line_number":242,"context_line":"                       body\u003dNone):"},{"line_number":243,"context_line":"        \"\"\"Proxy a request to a staging host."},{"line_number":244,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"65c9211e_3bc490a9","line":241,"range":{"start_line":241,"start_character":45,"end_line":241,"end_character":46},"in_reply_to":"0e092570_82166bb6","updated":"2025-05-26 17:52:23.000000000","message":"Acknowledged","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"76ca06597cfaab8f3ba0f8384a86ce771326f185","unresolved":true,"context_lines":[{"line_number":253,"context_line":""},{"line_number":254,"context_line":"        :param image: The Image from the repo"},{"line_number":255,"context_line":"        :param req: The webob.Request from the current request"},{"line_number":256,"context_line":"        :param host: URL to use for redirecting to host"},{"line_number":257,"context_line":"        :param body: The request body or None"},{"line_number":258,"context_line":"        :returns: The result from the remote host"},{"line_number":259,"context_line":"        :raises: webob.exc.HTTPClientError matching the remote\u0027s error, or"}],"source_content_type":"text/x-python","patch_set":1,"id":"929e3c99_ea276136","line":256,"range":{"start_line":256,"start_character":28,"end_line":256,"end_character":31},"updated":"2025-05-26 15:30:34.000000000","message":"Is it a URL or a property name?","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a3f8d1738e32c6ad3918e5df4d4a6bf183c0ec28","unresolved":false,"context_lines":[{"line_number":253,"context_line":""},{"line_number":254,"context_line":"        :param image: The Image from the repo"},{"line_number":255,"context_line":"        :param req: The webob.Request from the current request"},{"line_number":256,"context_line":"        :param host: URL to use for redirecting to host"},{"line_number":257,"context_line":"        :param body: The request body or None"},{"line_number":258,"context_line":"        :returns: The result from the remote host"},{"line_number":259,"context_line":"        :raises: webob.exc.HTTPClientError matching the remote\u0027s error, or"}],"source_content_type":"text/x-python","patch_set":1,"id":"71c01330_0f6e93fd","line":256,"range":{"start_line":256,"start_character":28,"end_line":256,"end_character":31},"in_reply_to":"929e3c99_ea276136","updated":"2025-05-26 17:52:23.000000000","message":"its a property name","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d8ccb50b314b21facec39b9f8b08a6a912d554b","unresolved":true,"context_lines":[{"line_number":439,"context_line":"            # import request to that worker with the user\u0027s token, as if"},{"line_number":440,"context_line":"            # they had called it themselves."},{"line_number":441,"context_line":"            host_key \u003d \u0027os_glance_stage_host\u0027"},{"line_number":442,"context_line":"            return self._proxy_request(image, req, host_key, body\u003dbody)"},{"line_number":443,"context_line":""},{"line_number":444,"context_line":"        task_input \u003d {\u0027image_id\u0027: image_id,"},{"line_number":445,"context_line":"                      \u0027import_req\u0027: body,"}],"source_content_type":"text/x-python","patch_set":3,"id":"b583dea1_49c2da16","line":442,"updated":"2025-05-27 14:45:03.000000000","message":"(see below)\n\nYour delete code should have the same semantics as this ... proxy the delete to the other node and return.","commit_id":"8d2ec7a35b063057c0bb82a264cf62d98b426757"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"def4f52d654b1be3d013a4f417a5fcfd4821fd1f","unresolved":false,"context_lines":[{"line_number":439,"context_line":"            # import request to that worker with the user\u0027s token, as if"},{"line_number":440,"context_line":"            # they had called it themselves."},{"line_number":441,"context_line":"            host_key \u003d \u0027os_glance_stage_host\u0027"},{"line_number":442,"context_line":"            return self._proxy_request(image, req, host_key, body\u003dbody)"},{"line_number":443,"context_line":""},{"line_number":444,"context_line":"        task_input \u003d {\u0027image_id\u0027: image_id,"},{"line_number":445,"context_line":"                      \u0027import_req\u0027: body,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ce4448c3_5f6096e8","line":442,"in_reply_to":"b583dea1_49c2da16","updated":"2025-05-29 07:50:41.000000000","message":"Done","commit_id":"8d2ec7a35b063057c0bb82a264cf62d98b426757"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d8ccb50b314b21facec39b9f8b08a6a912d554b","unresolved":true,"context_lines":[{"line_number":886,"context_line":"                registry.cancel_operation(operation_id)"},{"line_number":887,"context_line":"                # NOTE(abhishekk): Waiting for some time to ensure that"},{"line_number":888,"context_line":"                # hash operation is cancelled on the remote worker."},{"line_number":889,"context_line":"                time.sleep(5)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"            # NOTE(abhishekk): Delete the data from staging area"},{"line_number":892,"context_line":"            if CONF.enabled_backends:"}],"source_content_type":"text/x-python","patch_set":3,"id":"056fbba6_ae0d90f9","line":889,"updated":"2025-05-27 14:45:03.000000000","message":"This is, of course, a giant race condition, and it shouldn\u0027t be necessary, IMHO. Proxy the delete to the remote side and let it cancel, and then finish the delete itself. We should return the result of L881 here and not proceed further.","commit_id":"8d2ec7a35b063057c0bb82a264cf62d98b426757"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"def4f52d654b1be3d013a4f417a5fcfd4821fd1f","unresolved":false,"context_lines":[{"line_number":886,"context_line":"                registry.cancel_operation(operation_id)"},{"line_number":887,"context_line":"                # NOTE(abhishekk): Waiting for some time to ensure that"},{"line_number":888,"context_line":"                # hash operation is cancelled on the remote worker."},{"line_number":889,"context_line":"                time.sleep(5)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"            # NOTE(abhishekk): Delete the data from staging area"},{"line_number":892,"context_line":"            if CONF.enabled_backends:"}],"source_content_type":"text/x-python","patch_set":3,"id":"13bfb126_344a09a7","line":889,"in_reply_to":"056fbba6_ae0d90f9","updated":"2025-05-29 07:50:41.000000000","message":"Done","commit_id":"8d2ec7a35b063057c0bb82a264cf62d98b426757"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffa87bb04c13baa0403abc7380d99f784f83b167","unresolved":true,"context_lines":[{"line_number":261,"context_line":"        \"\"\""},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        stage_host \u003d image.extra_properties.get(\u0027os_glance_stage_host\u0027)"},{"line_number":264,"context_line":"        hash_op_host \u003d image.extra_properties.get(\u0027os_glance_hash_op_host\u0027)"},{"line_number":265,"context_line":"        LOG.info(_LI(\u0027Proxying %s request to host %s \u0027"},{"line_number":266,"context_line":"                     \u0027which has image staged\u0027),"},{"line_number":267,"context_line":"                 req.method, stage_host)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9dd8e1bb_f526f3ca","line":264,"updated":"2025-05-29 13:49:40.000000000","message":"FWIW, I liked what you had before (although I would have s/host/key/), which is passing the key into this function instead of embedding that logic here. It makes it more useful in the future I think. Why did you change it?","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ad2da9d8f99acbcf463a6d81e69df48f2008a346","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        \"\"\""},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        stage_host \u003d image.extra_properties.get(\u0027os_glance_stage_host\u0027)"},{"line_number":264,"context_line":"        hash_op_host \u003d image.extra_properties.get(\u0027os_glance_hash_op_host\u0027)"},{"line_number":265,"context_line":"        LOG.info(_LI(\u0027Proxying %s request to host %s \u0027"},{"line_number":266,"context_line":"                     \u0027which has image staged\u0027),"},{"line_number":267,"context_line":"                 req.method, stage_host)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1978b686_0f2fdd59","line":264,"in_reply_to":"9dd8e1bb_f526f3ca","updated":"2025-05-29 15:00:19.000000000","message":"Ack, I changed it because cyril asked me to make a host positional argument here rather than passing host/key as kwargs. I will make the changes as per suggestion like calling is_proxyable form here and return the expected host which can be used to proxy the request!","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffa87bb04c13baa0403abc7380d99f784f83b167","unresolved":true,"context_lines":[{"line_number":272,"context_line":"            url \u003d \u0027%s%s\u0027 % (hash_op_host, req.path)"},{"line_number":273,"context_line":"        else:"},{"line_number":274,"context_line":"            LOG.debug(\"Request can not be proxied to any host\")"},{"line_number":275,"context_line":"            return image.image_id"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"        request_method \u003d getattr(client, req.method.lower())"},{"line_number":278,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"d41ced25_d3e3568e","line":275,"updated":"2025-05-29 13:49:40.000000000","message":"Maybe I\u0027ll see below, but.. shouldn\u0027t this raise? (later: I see)","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ad2da9d8f99acbcf463a6d81e69df48f2008a346","unresolved":false,"context_lines":[{"line_number":272,"context_line":"            url \u003d \u0027%s%s\u0027 % (hash_op_host, req.path)"},{"line_number":273,"context_line":"        else:"},{"line_number":274,"context_line":"            LOG.debug(\"Request can not be proxied to any host\")"},{"line_number":275,"context_line":"            return image.image_id"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"        request_method \u003d getattr(client, req.method.lower())"},{"line_number":278,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3673dc90_5cbe05df","line":275,"in_reply_to":"d41ced25_d3e3568e","updated":"2025-05-29 15:00:19.000000000","message":"Acknowledged","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffa87bb04c13baa0403abc7380d99f784f83b167","unresolved":true,"context_lines":[{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        return ("},{"line_number":319,"context_line":"            (stage_host and stage_host !\u003d self.self_url) or"},{"line_number":320,"context_line":"            (hash_op_host and hash_op_host !\u003d self.self_url))"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    @utils.mutating"},{"line_number":323,"context_line":"    def import_image(self, req, image_id, body):"}],"source_content_type":"text/x-python","patch_set":5,"id":"df91e1dc_d25d99f8","line":320,"updated":"2025-05-29 13:49:40.000000000","message":"Same here... better to pass the key, IMHO.","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ad2da9d8f99acbcf463a6d81e69df48f2008a346","unresolved":false,"context_lines":[{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        return ("},{"line_number":319,"context_line":"            (stage_host and stage_host !\u003d self.self_url) or"},{"line_number":320,"context_line":"            (hash_op_host and hash_op_host !\u003d self.self_url))"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    @utils.mutating"},{"line_number":323,"context_line":"    def import_image(self, req, image_id, body):"}],"source_content_type":"text/x-python","patch_set":5,"id":"dc743185_c045dd43","line":320,"in_reply_to":"df91e1dc_d25d99f8","updated":"2025-05-29 15:00:19.000000000","message":"Acknowledged","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffa87bb04c13baa0403abc7380d99f784f83b167","unresolved":true,"context_lines":[{"line_number":875,"context_line":"                # NOTE(danms): Image is staged on another worker; proxy the"},{"line_number":876,"context_line":"                # delete request to that worker with the user\u0027s token, as if"},{"line_number":877,"context_line":"                # they had called it themselves."},{"line_number":878,"context_line":"                image \u003d self._delete_image_on_remote(image, req)"},{"line_number":879,"context_line":"                if image is None:"},{"line_number":880,"context_line":"                    # Delete was proxied, so we are done here."},{"line_number":881,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a38f0f7_218fdaa3","line":878,"updated":"2025-05-29 13:49:40.000000000","message":"Aha, so we already have delete proxying and embedding the this-or-that key logic in the under layers makes this just work.\n\nCould we at least embed the this-or-that logic on one place? What if we make `is_proxyable()` check the two (or more later) keys, and if it finds one, it returns the value? That will be truthy for L874 and then `_proxy_request_to_host()` can also call `is_proxyable()` but use the result instead of looking up the keys itself? That way, we only have one place where we decide which keys to examine.","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ad2da9d8f99acbcf463a6d81e69df48f2008a346","unresolved":false,"context_lines":[{"line_number":875,"context_line":"                # NOTE(danms): Image is staged on another worker; proxy the"},{"line_number":876,"context_line":"                # delete request to that worker with the user\u0027s token, as if"},{"line_number":877,"context_line":"                # they had called it themselves."},{"line_number":878,"context_line":"                image \u003d self._delete_image_on_remote(image, req)"},{"line_number":879,"context_line":"                if image is None:"},{"line_number":880,"context_line":"                    # Delete was proxied, so we are done here."},{"line_number":881,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":5,"id":"963da4ff_cd435003","line":878,"in_reply_to":"5a38f0f7_218fdaa3","updated":"2025-05-29 15:00:19.000000000","message":"Acknowledged","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffa87bb04c13baa0403abc7380d99f784f83b167","unresolved":true,"context_lines":[{"line_number":885,"context_line":"            # the hash"},{"line_number":886,"context_line":"            operation_id \u003d image.extra_properties.get(\u0027os_glance_import_task\u0027)"},{"line_number":887,"context_line":"            hash_host \u003d image.extra_properties.get(\u0027os_glance_hash_op_host\u0027)"},{"line_number":888,"context_line":"            if hash_host and operation_id:"},{"line_number":889,"context_line":"                tracker.cancel_operation(operation_id)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"            # NOTE(abhishekk): Delete the data from staging area"}],"source_content_type":"text/x-python","patch_set":5,"id":"39c8adc5_aaec1b9a","line":888,"updated":"2025-05-29 13:49:40.000000000","message":"Why do we need to look at `hash_host` here? If we\u0027re already on the same host (either because we got proxied to, or we\u0027re an AIO install), we can just cancel right? Are you trying to avoid raising \"no such operation to cancel\" here? Might be better to just catch that and LOG. It also doesn\u0027t check that hash_host is _us_ so if we happened to get asked to cancel a task we\u0027re not running, we\u0027ll raise here anyway right?","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ad2da9d8f99acbcf463a6d81e69df48f2008a346","unresolved":false,"context_lines":[{"line_number":885,"context_line":"            # the hash"},{"line_number":886,"context_line":"            operation_id \u003d image.extra_properties.get(\u0027os_glance_import_task\u0027)"},{"line_number":887,"context_line":"            hash_host \u003d image.extra_properties.get(\u0027os_glance_hash_op_host\u0027)"},{"line_number":888,"context_line":"            if hash_host and operation_id:"},{"line_number":889,"context_line":"                tracker.cancel_operation(operation_id)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"            # NOTE(abhishekk): Delete the data from staging area"}],"source_content_type":"text/x-python","patch_set":5,"id":"47a74900_492a4862","line":888,"in_reply_to":"39c8adc5_aaec1b9a","updated":"2025-05-29 15:00:19.000000000","message":"ack","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffa87bb04c13baa0403abc7380d99f784f83b167","unresolved":true,"context_lines":[{"line_number":1718,"context_line":"class ResponseSerializer(wsgi.JSONResponseSerializer):"},{"line_number":1719,"context_line":"    # These properties will be filtered out from the response and not"},{"line_number":1720,"context_line":"    # exposed to the client"},{"line_number":1721,"context_line":"    _hidden_properties \u003d [\u0027os_glance_stage_host\u0027]"},{"line_number":1722,"context_line":""},{"line_number":1723,"context_line":"    def __init__(self, schema\u003dNone, location_schema\u003dNone):"},{"line_number":1724,"context_line":"        super(ResponseSerializer, self).__init__()"}],"source_content_type":"text/x-python","patch_set":5,"id":"2ada853e_26d5535a","line":1721,"updated":"2025-05-29 13:49:40.000000000","message":"Need to add the new key here","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ad2da9d8f99acbcf463a6d81e69df48f2008a346","unresolved":false,"context_lines":[{"line_number":1718,"context_line":"class ResponseSerializer(wsgi.JSONResponseSerializer):"},{"line_number":1719,"context_line":"    # These properties will be filtered out from the response and not"},{"line_number":1720,"context_line":"    # exposed to the client"},{"line_number":1721,"context_line":"    _hidden_properties \u003d [\u0027os_glance_stage_host\u0027]"},{"line_number":1722,"context_line":""},{"line_number":1723,"context_line":"    def __init__(self, schema\u003dNone, location_schema\u003dNone):"},{"line_number":1724,"context_line":"        super(ResponseSerializer, self).__init__()"}],"source_content_type":"text/x-python","patch_set":5,"id":"69156814_8dda280e","line":1721,"in_reply_to":"2ada853e_26d5535a","updated":"2025-05-29 15:00:19.000000000","message":"Acknowledged","commit_id":"d80be2d834e3cf4b9c1cdac2c9bcb20197644cb6"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a00c96e62533f216dcd4ed55b10af3f866402ec7","unresolved":false,"context_lines":[{"line_number":877,"context_line":"            operation_id \u003d image.extra_properties.get(\u0027os_glance_import_task\u0027)"},{"line_number":878,"context_line":"            os_hashing_host \u003d image.extra_properties.get("},{"line_number":879,"context_line":"                \u0027os_glance_hash_op_host\u0027)"},{"line_number":880,"context_line":"            if operation_id and os_hashing_host:"},{"line_number":881,"context_line":"                tracker.cancel_operation(operation_id)"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"            # NOTE(abhishekk): Delete the data from staging area"}],"source_content_type":"text/x-python","patch_set":8,"id":"02c4019f_bf184fe7","line":880,"range":{"start_line":880,"start_character":32,"end_line":880,"end_character":47},"updated":"2025-06-09 15:29:57.000000000","message":"This needs to be check so that other import operations will not have regression if image is deleted while those are in progress.","commit_id":"0057236a239c91480b1efa76108a0419548371da"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"1eae5eb43a005b43544e8ca57d622e788ae80e26","unresolved":true,"context_lines":[{"line_number":241,"context_line":"                       \u0027keys\u0027: \u0027,\u0027.join(changed)})"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def _proxy_request_to_host(self, image, req, body\u003dNone):"},{"line_number":244,"context_line":"        \"\"\"Proxy a request to a respected host."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        When an image was staged on another worker or Location add workflow"},{"line_number":247,"context_line":"        having hash calculation operation is in progress, that worker may"}],"source_content_type":"text/x-python","patch_set":10,"id":"63559e68_43f2d54a","line":244,"range":{"start_line":244,"start_character":32,"end_line":244,"end_character":41},"updated":"2025-06-13 18:37:34.000000000","message":"What is a \"respected\" host? Is that some terminology I should be aware of?","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"57ec8d2689aa395599142185bb3fee1ed8849af3","unresolved":true,"context_lines":[{"line_number":241,"context_line":"                       \u0027keys\u0027: \u0027,\u0027.join(changed)})"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def _proxy_request_to_host(self, image, req, body\u003dNone):"},{"line_number":244,"context_line":"        \"\"\"Proxy a request to a respected host."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        When an image was staged on another worker or Location add workflow"},{"line_number":247,"context_line":"        having hash calculation operation is in progress, that worker may"}],"source_content_type":"text/x-python","patch_set":10,"id":"f7e662d1_19262ca5","line":244,"range":{"start_line":244,"start_character":32,"end_line":244,"end_character":41},"in_reply_to":"63559e68_43f2d54a","updated":"2025-06-13 19:58:02.000000000","message":"I think \"respective\" is what Abhi meant here.","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0e06825cfd4208d4a3581bc2a0abebfd61d351c7","unresolved":true,"context_lines":[{"line_number":241,"context_line":"                       \u0027keys\u0027: \u0027,\u0027.join(changed)})"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def _proxy_request_to_host(self, image, req, body\u003dNone):"},{"line_number":244,"context_line":"        \"\"\"Proxy a request to a respected host."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        When an image was staged on another worker or Location add workflow"},{"line_number":247,"context_line":"        having hash calculation operation is in progress, that worker may"}],"source_content_type":"text/x-python","patch_set":10,"id":"8cbde880_c4a5598d","line":244,"range":{"start_line":244,"start_character":32,"end_line":244,"end_character":41},"in_reply_to":"f7e662d1_19262ca5","updated":"2025-06-16 06:25:54.000000000","message":"yep, respective, let me know if I should respin the patch","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"1eae5eb43a005b43544e8ca57d622e788ae80e26","unresolved":true,"context_lines":[{"line_number":878,"context_line":"            os_hashing_host \u003d image.extra_properties.get("},{"line_number":879,"context_line":"                \u0027os_glance_hash_op_host\u0027)"},{"line_number":880,"context_line":"            if operation_id and os_hashing_host:"},{"line_number":881,"context_line":"                tracker.cancel_operation(operation_id)"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"            # NOTE(abhishekk): Delete the data from staging area"},{"line_number":884,"context_line":"            if CONF.enabled_backends:"}],"source_content_type":"text/x-python","patch_set":10,"id":"634492ae_8f850442","line":881,"range":{"start_line":881,"start_character":24,"end_line":881,"end_character":40},"updated":"2025-06-13 18:37:34.000000000","message":"The comment states that we are canceling *and* signaling, but we\u0027re only canceling here. Are we not going to spend 60 seconds in cancel_operation() waiting for the lock file to be removed?","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"4ff6356a2b7f4a3536d09222bd386b848acfa853","unresolved":true,"context_lines":[{"line_number":878,"context_line":"            os_hashing_host \u003d image.extra_properties.get("},{"line_number":879,"context_line":"                \u0027os_glance_hash_op_host\u0027)"},{"line_number":880,"context_line":"            if operation_id and os_hashing_host:"},{"line_number":881,"context_line":"                tracker.cancel_operation(operation_id)"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"            # NOTE(abhishekk): Delete the data from staging area"},{"line_number":884,"context_line":"            if CONF.enabled_backends:"}],"source_content_type":"text/x-python","patch_set":10,"id":"8408922c_8c00d8b1","line":881,"range":{"start_line":881,"start_character":24,"end_line":881,"end_character":40},"in_reply_to":"396f0c74_870a45d4","updated":"2025-06-13 21:20:03.000000000","message":"My understanding is that cancel_operation() waits for the lock file to be removed for 60 * 0.5 seconds (so, 30 seconds, my bad). But the lock file is removed by signal_finished(), and I\u0027m not sure how we end up calling signal_finished() when doing a deletion.","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"57ec8d2689aa395599142185bb3fee1ed8849af3","unresolved":true,"context_lines":[{"line_number":878,"context_line":"            os_hashing_host \u003d image.extra_properties.get("},{"line_number":879,"context_line":"                \u0027os_glance_hash_op_host\u0027)"},{"line_number":880,"context_line":"            if operation_id and os_hashing_host:"},{"line_number":881,"context_line":"                tracker.cancel_operation(operation_id)"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"            # NOTE(abhishekk): Delete the data from staging area"},{"line_number":884,"context_line":"            if CONF.enabled_backends:"}],"source_content_type":"text/x-python","patch_set":10,"id":"396f0c74_870a45d4","line":881,"range":{"start_line":881,"start_character":24,"end_line":881,"end_character":40},"in_reply_to":"634492ae_8f850442","updated":"2025-06-13 19:58:02.000000000","message":"...We\u0027re canceling. We do that by signaling that we want it to be canceled, and waiting for it to be acked. It won\u0027t wait 60s always, no.\n\nI guess I\u0027m not sure what the confusion or concern is.","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7717c4ce8438d41c410c8fd68808286d0704eb7a","unresolved":true,"context_lines":[{"line_number":878,"context_line":"            os_hashing_host \u003d image.extra_properties.get("},{"line_number":879,"context_line":"                \u0027os_glance_hash_op_host\u0027)"},{"line_number":880,"context_line":"            if operation_id and os_hashing_host:"},{"line_number":881,"context_line":"                tracker.cancel_operation(operation_id)"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"            # NOTE(abhishekk): Delete the data from staging area"},{"line_number":884,"context_line":"            if CONF.enabled_backends:"}],"source_content_type":"text/x-python","patch_set":10,"id":"caf101a2_170640fa","line":881,"range":{"start_line":881,"start_character":24,"end_line":881,"end_character":40},"in_reply_to":"8408922c_8c00d8b1","updated":"2025-06-16 13:48:39.000000000","message":"...in the import task where we\u0027re doing the hashing. I added a comment pointer.","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"}],"glance/async_/flows/location_import.py":[{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"76ca06597cfaab8f3ba0f8384a86ce771326f185","unresolved":true,"context_lines":[{"line_number":66,"context_line":"        current_checksum \u003d hashlib.md5(usedforsecurity\u003dFalse)"},{"line_number":67,"context_line":"        for chunk in image.get_data():"},{"line_number":68,"context_line":"            is_cancelled \u003d registry.get_registry().get("},{"line_number":69,"context_line":"                operation_id, {}).get(\u0027cancelled\u0027, False)"},{"line_number":70,"context_line":"            if is_cancelled:"},{"line_number":71,"context_line":"                # NOTE(abhishekk): If the operation is cancelled,"},{"line_number":72,"context_line":"                # we should exit early without calculating the hash."}],"source_content_type":"text/x-python","patch_set":1,"id":"6bb27df5_8684c263","line":69,"range":{"start_line":69,"start_character":38,"end_line":69,"end_character":39},"updated":"2025-05-26 15:30:34.000000000","message":"OK maybe there could be a \"is_cancelled\" function in registry.py?","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a3f8d1738e32c6ad3918e5df4d4a6bf183c0ec28","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        current_checksum \u003d hashlib.md5(usedforsecurity\u003dFalse)"},{"line_number":67,"context_line":"        for chunk in image.get_data():"},{"line_number":68,"context_line":"            is_cancelled \u003d registry.get_registry().get("},{"line_number":69,"context_line":"                operation_id, {}).get(\u0027cancelled\u0027, False)"},{"line_number":70,"context_line":"            if is_cancelled:"},{"line_number":71,"context_line":"                # NOTE(abhishekk): If the operation is cancelled,"},{"line_number":72,"context_line":"                # we should exit early without calculating the hash."}],"source_content_type":"text/x-python","patch_set":1,"id":"eab2112c_53558828","line":69,"range":{"start_line":69,"start_character":38,"end_line":69,"end_character":39},"in_reply_to":"6bb27df5_8684c263","updated":"2025-05-26 17:52:23.000000000","message":"Acknowledged","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"76ca06597cfaab8f3ba0f8384a86ce771326f185","unresolved":true,"context_lines":[{"line_number":100,"context_line":"                    target\u003dself._calculate_hash,"},{"line_number":101,"context_line":"                    args\u003d(image, operation_id)"},{"line_number":102,"context_line":"                )"},{"line_number":103,"context_line":"                process.start()"},{"line_number":104,"context_line":"            except IOError as e:"},{"line_number":105,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":106,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":1,"id":"000c6c4f_3f6f0d06","line":103,"range":{"start_line":103,"start_character":24,"end_line":103,"end_character":29},"updated":"2025-05-26 15:30:34.000000000","message":"Why do we use multiprocessing.Process() here? Just to be able to use multiprocessing.Lock() in registry?","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a3f8d1738e32c6ad3918e5df4d4a6bf183c0ec28","unresolved":false,"context_lines":[{"line_number":100,"context_line":"                    target\u003dself._calculate_hash,"},{"line_number":101,"context_line":"                    args\u003d(image, operation_id)"},{"line_number":102,"context_line":"                )"},{"line_number":103,"context_line":"                process.start()"},{"line_number":104,"context_line":"            except IOError as e:"},{"line_number":105,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":106,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":1,"id":"b03a0957_018dbedc","line":103,"range":{"start_line":103,"start_character":24,"end_line":103,"end_character":29},"in_reply_to":"000c6c4f_3f6f0d06","updated":"2025-05-26 17:52:23.000000000","message":"Removed","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ff3f4d61f37394cceb6a5185e6ebe5183a043c65","unresolved":true,"context_lines":[{"line_number":70,"context_line":"                LOG.debug(\"Hash calculation for image %s is cancelled\","},{"line_number":71,"context_line":"                          self.image_id)"},{"line_number":72,"context_line":"                registry.delete_operation(operation_id)"},{"line_number":73,"context_line":"                return"},{"line_number":74,"context_line":"            if chunk is None:"},{"line_number":75,"context_line":"                break"},{"line_number":76,"context_line":"            current_checksum.update(chunk)"}],"source_content_type":"text/x-python","patch_set":2,"id":"d5fece6d_dc03c527","line":73,"range":{"start_line":73,"start_character":16,"end_line":73,"end_character":22},"updated":"2025-05-27 10:29:03.000000000","message":"Not sure this is helping here\n\nMay 27 07:30:31.413692 np0040903853 glance-api[102629]: DEBUG glance.async_.flows.location_import [-] Hash calculation for image 45453f21-b17f-499c-bb09-11700c61db61 is cancelled {{(pid\u003d102629) _calculate_hash /opt/stack/glance/glance/async_/flows/location_import.py:70}}\nMay 27 07:30:31.414148 np0040903853 glance-api[102629]: WARNING glance_store._drivers.rbd [None req-bdf43ab7-469e-44b4-b78b-02014d10dca2 tempest-ListImageFiltersTestJSON-616982599 tempest-ListImageFiltersTestJSON-616982599-project-member] Remove image 45453f21-b17f-499c-bb09-11700c61db61 failed. It is in use.: rbd.ImageBusy: [errno 16] RBD image is busy (error removing image)\nMay 27 07:30:31.418849 np0040903853 glance-api[102629]: WARNING glance.api.v2.images [None req-bdf43ab7-469e-44b4-b78b-02014d10dca2 tempest-ListImageFiltersTestJSON-616982599 tempest-ListImageFiltersTestJSON-616982599-project-member] Image 45453f21-b17f-499c-bb09-11700c61db61 could not be deleted because it is in use: The image cannot be deleted because it is in use through the backend store outside of Glance.: glance_store.exceptions.InUseByStore: The image cannot be deleted because it is in use through the backend store outside of Glance.\nMay 27 07:30:31.420138 np0040903853 glance-api[102629]: INFO eventlet.wsgi.server [None req-bdf43ab7-469e-44b4-b78b-02014d10dca2 tempest-ListImageFiltersTestJSON-616982599 tempest-ListImageFiltersTestJSON-616982599-project-member] 10.176.194.42,10.176.194.42,10.176.194.42 - - [27/May/2025 07:30:31] \"DELETE /v2/images/45453f21-b17f-499c-bb09-11700c61db61 HTTP/1.1\" 409 512 0.956740","commit_id":"556fa9d149dc3fb3ece2597e7be775eca83c19e0"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"def4f52d654b1be3d013a4f417a5fcfd4821fd1f","unresolved":false,"context_lines":[{"line_number":70,"context_line":"                LOG.debug(\"Hash calculation for image %s is cancelled\","},{"line_number":71,"context_line":"                          self.image_id)"},{"line_number":72,"context_line":"                registry.delete_operation(operation_id)"},{"line_number":73,"context_line":"                return"},{"line_number":74,"context_line":"            if chunk is None:"},{"line_number":75,"context_line":"                break"},{"line_number":76,"context_line":"            current_checksum.update(chunk)"}],"source_content_type":"text/x-python","patch_set":2,"id":"797a8ef0_159ad4ea","line":73,"range":{"start_line":73,"start_character":16,"end_line":73,"end_character":22},"in_reply_to":"0f98b657_cffa2855","updated":"2025-05-29 07:50:41.000000000","message":"Done","commit_id":"556fa9d149dc3fb3ece2597e7be775eca83c19e0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d8ccb50b314b21facec39b9f8b08a6a912d554b","unresolved":true,"context_lines":[{"line_number":70,"context_line":"                LOG.debug(\"Hash calculation for image %s is cancelled\","},{"line_number":71,"context_line":"                          self.image_id)"},{"line_number":72,"context_line":"                registry.delete_operation(operation_id)"},{"line_number":73,"context_line":"                return"},{"line_number":74,"context_line":"            if chunk is None:"},{"line_number":75,"context_line":"                break"},{"line_number":76,"context_line":"            current_checksum.update(chunk)"}],"source_content_type":"text/x-python","patch_set":2,"id":"0f98b657_cffa2855","line":73,"range":{"start_line":73,"start_character":16,"end_line":73,"end_character":22},"in_reply_to":"d5fece6d_dc03c527","updated":"2025-05-27 14:45:03.000000000","message":"Both workers will attempt the delete in the current form you have here, so maybe that\u0027s part of it. Also, if the data pipeline is blocked for more than 5 seconds (highly possible, especially in CI) then you\u0027ll continue to try to delete before it has been canceled.","commit_id":"556fa9d149dc3fb3ece2597e7be775eca83c19e0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"486fd9f14f2e6c903a094fd522079e149b14c525","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            except _HashCalculationCanceled as e:"},{"line_number":91,"context_line":"                LOG.debug(\u0027Hash calculation cancelled: %s\u0027,"},{"line_number":92,"context_line":"                          encodeutils.exception_to_unicode(e))"},{"line_number":93,"context_line":"                break"},{"line_number":94,"context_line":"            except IOError as e:"},{"line_number":95,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":96,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":8,"id":"a76f82c9_be8452a2","line":93,"updated":"2025-06-10 14:08:14.000000000","message":"So, this will just result in the hashing being stopped, but won\u0027t actually fail the import. I noticed while looking at the logs, that the image continues on to the rest of the import successfully even after we do this:\n```\nJun 09 19:51:11.469576 np0041068284 devstack@g-api.service[88351]: DEBUG glance.async_.flows.location_import [-] Hash calculation cancelled: Hash calculation for image 6379ffea-2660-4997-85e9-64d342ef9085 has been canceled {{(pid\u003d88351) _set_checksum_and_hash /opt/stack/glance/glance/async_/flows/location_import.py:91}}\nJun 09 19:51:11.491471 np0041068284 devstack@g-api.service[88351]: DEBUG glance_store.backend [-] Skipping store.set_acls... not implemented. {{(pid\u003d88351) set_acls /opt/stack/data/venv/lib/python3.12/site-packages/glance_store/backend.py:508}}\nJun 09 19:51:11.493370 np0041068284 devstack@g-api.service[88351]: DEBUG glance.async_.taskflow_executor [-] Task \u0027location_import-CalculateHash-6c3961b9-73f7-40ac-b520-c19fe2bb2e6e\u0027 (802986aa-6f1a-4982-8e34-87a1c665f176) transitioned into state \u0027SUCCESS\u0027 from state \u0027RUNNING\u0027 with result \u0027None\u0027 {{(pid\u003d88351) _task_receiver /opt/stack/data/venv/lib/python3.12/site-packages/taskflow/listeners/logging.py:182}}\nJun 09 19:51:11.494176 np0041068284 devstack@g-api.service[88351]: DEBUG glance.async_.taskflow_executor [-] Task \u0027location_import-CompleteTask-6c3961b9-73f7-40ac-b520-c19fe2bb2e6e\u0027 (0dc661a5-e66e-4317-b8b1-e7c1f4444cb5) transitioned into state \u0027RUNNING\u0027 from state \u0027PENDING\u0027 {{(pid\u003d88351) _task_receiver /opt/stack/data/venv/lib/python3.12/site-packages/taskflow/listeners/logging.py:194}}\nJun 09 19:51:11.496834 np0041068284 devstack@g-api.service[88351]: INFO glance.domain [-] Task [6c3961b9-73f7-40ac-b520-c19fe2bb2e6e] status changing from processing to success\nJun 09 19:51:11.512795 np0041068284 devstack@g-api.service[88351]: INFO glance.async_.flows.api_image_import [-] 6c3961b9-73f7-40ac-b520-c19fe2bb2e6e of location_import completed\nJun 09 19:51:11.513567 np0041068284 devstack@g-api.service[88351]: DEBUG glance.async_.taskflow_executor [-] Task \u0027location_import-CompleteTask-6c3961b9-73f7-40ac-b520-c19fe2bb2e6e\u0027 (0dc661a5-e66e-4317-b8b1-e7c1f4444cb5) transitioned into state \u0027SUCCESS\u0027 from state \u0027RUNNING\u0027 with result \u0027None\u0027 {{(pid\u003d88351) _task_receiver /opt/stack/data/venv/lib/python3.12/site-packages/taskflow/listeners/logging.py:182}}\nJun 09 19:51:11.514436 np0041068284 devstack@g-api.service[88351]: DEBUG glance.async_.taskflow_executor [-] Flow \u0027location_import\u0027 (48ba7855-deb5-440d-a5de-0db6078de104) transitioned into state \u0027SUCCESS\u0027 from state \u0027RUNNING\u0027 {{(pid\u003d88351) _flow_receiver /opt/stack/data/venv/lib/python3.12/site-packages/taskflow/listeners/logging.py:145}}\nJun 09 19:51:11.521494 np0041068284 devstack@g-api.service[88351]: WARNING glance.api.v2.images [None req-17627080-2f49-4274-86da-25af3c67c5f8 tempest-HashCalculationRemoteDeletionTest-110145301 tempest-HashCalculationRemoteDeletionTest-110145301-project-member] After upload to backend, deletion of staged image data has failed because it cannot be found at /tmp/staging//6379ffea-2660-4997-85e9-64d342ef9085\nJun 09 19:51:11.521954 np0041068284 devstack@g-api.service[88351]: WARNING glance.common.store_utils [None req-17627080-2f49-4274-86da-25af3c67c5f8 tempest-HashCalculationRemoteDeletionTest-110145301 tempest-HashCalculationRemoteDeletionTest-110145301-project-member] Deleting images from this store is not supported.: glance_store.exceptions.StoreDeleteNotSupported: Deleting images from this store is not supported.\nJun 09 19:51:11.557427 np0041068284 devstack@g-api.service[88351]: [pid: 88351|app: 0|req: 896/1796] 127.0.0.1 () {32 vars in 693 bytes} [Mon Jun  9 19:50:56 2025] DELETE /v2/images/6379ffea-2660-4997-85e9-64d342ef9085 \u003d\u003e generated 0 bytes in 15049 msecs (HTTP/1.1 204) 4 headers in 171 bytes (1 switches on core 0)\n```\nThe image *does* seem to be deleted after this, so maybe it doesn\u0027t matter, but it kinda seems like a bad idea to just let the import continue. If hashing is required, and the hashing is canceled for some reason, we shouldn\u0027t continue with the import right? Like, what if we detect cancelation for some erroneous reason, or we later add another reason/path to cancel this? I would think that if we\u0027re supposed to hash and can\u0027t, for whatever reason, then the import should fail. No?","commit_id":"0057236a239c91480b1efa76108a0419548371da"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0daae3a6f244794458b21bbe9415578ffbd9ad7e","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            except _HashCalculationCanceled as e:"},{"line_number":91,"context_line":"                LOG.debug(\u0027Hash calculation cancelled: %s\u0027,"},{"line_number":92,"context_line":"                          encodeutils.exception_to_unicode(e))"},{"line_number":93,"context_line":"                break"},{"line_number":94,"context_line":"            except IOError as e:"},{"line_number":95,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":96,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":8,"id":"2a8800e3_4fee76fa","line":93,"in_reply_to":"031b6d14_02121b65","updated":"2025-06-10 16:42:10.000000000","message":"image status is deleted in the database and if we initiate revert flow then it will fail while setting image state back to queued unless we explicitly handle it now","commit_id":"0057236a239c91480b1efa76108a0419548371da"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d52f5c612eeca853eb8f64ae6dc3caa2cc65ef9f","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            except _HashCalculationCanceled as e:"},{"line_number":91,"context_line":"                LOG.debug(\u0027Hash calculation cancelled: %s\u0027,"},{"line_number":92,"context_line":"                          encodeutils.exception_to_unicode(e))"},{"line_number":93,"context_line":"                break"},{"line_number":94,"context_line":"            except IOError as e:"},{"line_number":95,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":96,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":8,"id":"936d4921_88853bef","line":93,"in_reply_to":"2a8800e3_4fee76fa","updated":"2025-06-10 16:55:27.000000000","message":"What I mean is without doing revert, I think we end up with active,deleted\u003dTrue when we should actually end up with deleted,deleted\u003dTrue. But yes, I think we need to handle it explicitly.","commit_id":"0057236a239c91480b1efa76108a0419548371da"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cee2061755f2f0ce8d03d086f929c67ce3340969","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            except _HashCalculationCanceled as e:"},{"line_number":91,"context_line":"                LOG.debug(\u0027Hash calculation cancelled: %s\u0027,"},{"line_number":92,"context_line":"                          encodeutils.exception_to_unicode(e))"},{"line_number":93,"context_line":"                break"},{"line_number":94,"context_line":"            except IOError as e:"},{"line_number":95,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":96,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":8,"id":"031b6d14_02121b65","line":93,"in_reply_to":"35e9979e_6958c7ff","updated":"2025-06-10 15:47:04.000000000","message":"Right, but isn\u0027t that the desired behavior - to revert the task so the state goes back to the way it was? I know the state is *actually* not important in this case because we\u0027re focusing on delete, but in any other situation it would be very wrong for it to cancel the hashing and go straight to ACTIVE state, which is what it ends up doing here. In fact, I wonder if it\u0027s actually deleted\u003dTrue,status\u003dactive in the database after this?","commit_id":"0057236a239c91480b1efa76108a0419548371da"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"466c7cbe84056860dc3ba2bf378dc415ba192062","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            except _HashCalculationCanceled as e:"},{"line_number":91,"context_line":"                LOG.debug(\u0027Hash calculation cancelled: %s\u0027,"},{"line_number":92,"context_line":"                          encodeutils.exception_to_unicode(e))"},{"line_number":93,"context_line":"                break"},{"line_number":94,"context_line":"            except IOError as e:"},{"line_number":95,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":96,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ee5ea048_729f4e71","line":93,"in_reply_to":"936d4921_88853bef","updated":"2025-06-11 14:01:21.000000000","message":"Just FYI, at line #98 also even after certain retries we are simply continuing with warning and not failing","commit_id":"0057236a239c91480b1efa76108a0419548371da"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"e043af563aa7f6dcd87d5f3e10c64bb858bb3c4b","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            except _HashCalculationCanceled as e:"},{"line_number":91,"context_line":"                LOG.debug(\u0027Hash calculation cancelled: %s\u0027,"},{"line_number":92,"context_line":"                          encodeutils.exception_to_unicode(e))"},{"line_number":93,"context_line":"                break"},{"line_number":94,"context_line":"            except IOError as e:"},{"line_number":95,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":96,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":8,"id":"35e9979e_6958c7ff","line":93,"in_reply_to":"a76f82c9_be8452a2","updated":"2025-06-10 14:34:42.000000000","message":"If we fail the import it will initiate the revert flow which will be more complicated and might need changes to avoid any stack trace. If you are Ok with this flow then I don’t have any problem to implement it. Unfortunately we don’t have a way to abort the task in taskflow.","commit_id":"0057236a239c91480b1efa76108a0419548371da"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7e4d0731ae4ab2b7b6372cbe27db0caa8ac878d1","unresolved":false,"context_lines":[{"line_number":90,"context_line":"            except _HashCalculationCanceled as e:"},{"line_number":91,"context_line":"                LOG.debug(\u0027Hash calculation cancelled: %s\u0027,"},{"line_number":92,"context_line":"                          encodeutils.exception_to_unicode(e))"},{"line_number":93,"context_line":"                break"},{"line_number":94,"context_line":"            except IOError as e:"},{"line_number":95,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":96,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":8,"id":"1e501d03_431c9527","line":93,"in_reply_to":"ee5ea048_729f4e71","updated":"2025-06-11 15:07:45.000000000","message":"Done","commit_id":"0057236a239c91480b1efa76108a0419548371da"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5ef982b5b9d6114974433bc9cbfb1ebb9dad2d83","unresolved":true,"context_lines":[{"line_number":138,"context_line":"           state"},{"line_number":139,"context_line":"        \"\"\""},{"line_number":140,"context_line":"        try:"},{"line_number":141,"context_line":"            if os.path.exists(tracker.path_for_op(self.task_id)):"},{"line_number":142,"context_line":"                tracker.signal_finished(self.task_id)"},{"line_number":143,"context_line":"            image \u003d self.image_repo.get(self.image_id)"},{"line_number":144,"context_line":"            if image.status \u003d\u003d \u0027importing\u0027:"}],"source_content_type":"text/x-python","patch_set":9,"id":"00c6dd1c_ba1b0f6e","line":141,"updated":"2025-06-13 14:47:40.000000000","message":"I think it would be best if we don\u0027t bleed the details of how the tracker works into modules outside the tracker itself. (that probably means they should be _private, but...).\n\nI think just always running `signal_finished()` is fine here, even though we log a warning about it. Since this doesn\u0027t grab and hold the lock across the check/signal, it\u0027s a race anyway and thus not always reliable.","commit_id":"7081cc9592ccef209e753bd2dcadc79b144c7463"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"045e4b00a79de11bc44fa08dc8eecd8877afa891","unresolved":false,"context_lines":[{"line_number":138,"context_line":"           state"},{"line_number":139,"context_line":"        \"\"\""},{"line_number":140,"context_line":"        try:"},{"line_number":141,"context_line":"            if os.path.exists(tracker.path_for_op(self.task_id)):"},{"line_number":142,"context_line":"                tracker.signal_finished(self.task_id)"},{"line_number":143,"context_line":"            image \u003d self.image_repo.get(self.image_id)"},{"line_number":144,"context_line":"            if image.status \u003d\u003d \u0027importing\u0027:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3b633350_b0df674f","line":141,"in_reply_to":"00c6dd1c_ba1b0f6e","updated":"2025-06-13 14:59:17.000000000","message":"Acknowledged","commit_id":"7081cc9592ccef209e753bd2dcadc79b144c7463"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"1eae5eb43a005b43544e8ca57d622e788ae80e26","unresolved":true,"context_lines":[{"line_number":91,"context_line":"            except _HashCalculationCanceled as e:"},{"line_number":92,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":93,"context_line":"                    LOG.debug(\u0027Hash calculation cancelled: %s\u0027,"},{"line_number":94,"context_line":"                              encodeutils.exception_to_unicode(e))"},{"line_number":95,"context_line":"            except IOError as e:"},{"line_number":96,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":97,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":10,"id":"e899383e_6c2f87ac","line":94,"range":{"start_line":94,"start_character":30,"end_line":94,"end_character":41},"updated":"2025-06-13 18:37:34.000000000","message":"So here we *reraise* the exception, but where do we catch it?","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"4ff6356a2b7f4a3536d09222bd386b848acfa853","unresolved":false,"context_lines":[{"line_number":91,"context_line":"            except _HashCalculationCanceled as e:"},{"line_number":92,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":93,"context_line":"                    LOG.debug(\u0027Hash calculation cancelled: %s\u0027,"},{"line_number":94,"context_line":"                              encodeutils.exception_to_unicode(e))"},{"line_number":95,"context_line":"            except IOError as e:"},{"line_number":96,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":97,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":10,"id":"aed9e469_e7f78398","line":94,"range":{"start_line":94,"start_character":30,"end_line":94,"end_character":41},"in_reply_to":"218a0357_ec1bbd4e","updated":"2025-06-13 21:20:03.000000000","message":"Acknowledged","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"57ec8d2689aa395599142185bb3fee1ed8849af3","unresolved":true,"context_lines":[{"line_number":91,"context_line":"            except _HashCalculationCanceled as e:"},{"line_number":92,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":93,"context_line":"                    LOG.debug(\u0027Hash calculation cancelled: %s\u0027,"},{"line_number":94,"context_line":"                              encodeutils.exception_to_unicode(e))"},{"line_number":95,"context_line":"            except IOError as e:"},{"line_number":96,"context_line":"                LOG.debug(\u0027[%i/%i] Hash calculation failed due to %s\u0027,"},{"line_number":97,"context_line":"                          retries, CONF.http_retries,"}],"source_content_type":"text/x-python","patch_set":10,"id":"218a0357_ec1bbd4e","line":94,"range":{"start_line":94,"start_character":30,"end_line":94,"end_character":41},"in_reply_to":"e899383e_6c2f87ac","updated":"2025-06-13 19:58:02.000000000","message":"This is a Task(flow) which means we raise so that the taskflow executor sees that something failed and reverts all the steps in the flow to un-do what has been done. Same as, for example, L106 where we determine the hash doesn\u0027t match, so we raise in order for the flow executor to reverse everything that has been done so far since the operation failed.","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7717c4ce8438d41c410c8fd68808286d0704eb7a","unresolved":true,"context_lines":[{"line_number":113,"context_line":"                            \u0027as image data has been deleted from the \u0027"},{"line_number":114,"context_line":"                            \u0027backend\u0027), {\u0027image_id\u0027: self.image_id})"},{"line_number":115,"context_line":"            finally:"},{"line_number":116,"context_line":"                tracker.signal_finished(self.task_id)"},{"line_number":117,"context_line":"                image.extra_properties.pop(\u0027os_glance_hash_op_host\u0027, None)"},{"line_number":118,"context_line":"                self.image_repo.save(image)"},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"78487243_dd323212","line":116,"updated":"2025-06-16 13:48:39.000000000","message":"Here\u0027s where we signal that we\u0027re finished, which lets any threads waiting for us to finish know that it\u0027s done. Even if we fail before we get into this try..finally block, we also call it in `revert()`.","commit_id":"f84e4f0123306a9bb47846cd73fc78578e7825aa"}],"glance/registry.py":[{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"76ca06597cfaab8f3ba0f8384a86ce771326f185","unresolved":true,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def set_registry(registry_dict):"},{"line_number":49,"context_line":"    serialized \u003d pickle.dumps(registry_dict)"},{"line_number":50,"context_line":"    mc.set(REGISTRY_KEY, serialized)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7fa7d48c_c1244044","line":49,"range":{"start_line":49,"start_character":30,"end_line":49,"end_character":43},"updated":"2025-05-26 15:30:34.000000000","message":"Do we really need to use pickle here? It seems to work without pickle:\n\n    \u003e\u003e\u003e mc.set(\u0027operation-id-123\u0027, {\u0027cancelled\u0027:False})\n    True\n    \u003e\u003e\u003e mc.get(\u0027operation-id-123\u0027)\n    {\u0027cancelled\u0027: False}\n\nIs there some subtlety I\u0027m missing?","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a3f8d1738e32c6ad3918e5df4d4a6bf183c0ec28","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def set_registry(registry_dict):"},{"line_number":49,"context_line":"    serialized \u003d pickle.dumps(registry_dict)"},{"line_number":50,"context_line":"    mc.set(REGISTRY_KEY, serialized)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"be24dd6d_2b2ee8fe","line":49,"range":{"start_line":49,"start_character":30,"end_line":49,"end_character":43},"in_reply_to":"6548a64f_71845d2f","updated":"2025-05-26 17:52:23.000000000","message":"No worries, removed use of pickle and used jsonutils from oslo_serialization instread","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"fcb53bd4c4d61c928a0762788822932f42c59e73","unresolved":true,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def set_registry(registry_dict):"},{"line_number":49,"context_line":"    serialized \u003d pickle.dumps(registry_dict)"},{"line_number":50,"context_line":"    mc.set(REGISTRY_KEY, serialized)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"41d88dce_cbdf9c50","line":49,"range":{"start_line":49,"start_character":30,"end_line":49,"end_character":43},"in_reply_to":"6548a64f_71845d2f","updated":"2025-05-26 17:48:06.000000000","message":"Oh yeah I was testing something slightly different, sorry for the noise.","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"15c52dde56822c0a40d07a99a5506d95bbf2c702","unresolved":true,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def set_registry(registry_dict):"},{"line_number":49,"context_line":"    serialized \u003d pickle.dumps(registry_dict)"},{"line_number":50,"context_line":"    mc.set(REGISTRY_KEY, serialized)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"6548a64f_71845d2f","line":49,"range":{"start_line":49,"start_character":30,"end_line":49,"end_character":43},"in_reply_to":"7fa7d48c_c1244044","updated":"2025-05-26 15:42:58.000000000","message":"File \"/opt/stack/glance/glance/registry.py\", line 59, in register_operation\n    registry[operation_id] \u003d {\u0027cancelled\u0027: False}\n\n    TypeError: \u0027bytes\u0027 object does not support item assignment","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"76ca06597cfaab8f3ba0f8384a86ce771326f185","unresolved":true,"context_lines":[{"line_number":68,"context_line":"    registry \u003d get_registry()"},{"line_number":69,"context_line":"    if operation_id in registry:"},{"line_number":70,"context_line":"        registry[operation_id][\u0027cancelled\u0027] \u003d True"},{"line_number":71,"context_line":"        set_registry(registry)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def delete_operation(operation_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"0cbaf4e8_6b38fd3a","line":71,"range":{"start_line":71,"start_character":29,"end_line":71,"end_character":30},"updated":"2025-05-26 15:30:34.000000000","message":"It would be good to add an \"else:\" (or use try/except) to be able to print a log when we try to cancel an operation which does not exist (even though ideally, that would never happen :p).","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a3f8d1738e32c6ad3918e5df4d4a6bf183c0ec28","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    registry \u003d get_registry()"},{"line_number":69,"context_line":"    if operation_id in registry:"},{"line_number":70,"context_line":"        registry[operation_id][\u0027cancelled\u0027] \u003d True"},{"line_number":71,"context_line":"        set_registry(registry)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def delete_operation(operation_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"efc286eb_bcae93c0","line":71,"range":{"start_line":71,"start_character":29,"end_line":71,"end_character":30},"in_reply_to":"0cbaf4e8_6b38fd3a","updated":"2025-05-26 17:52:23.000000000","message":"Acknowledged","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"76ca06597cfaab8f3ba0f8384a86ce771326f185","unresolved":true,"context_lines":[{"line_number":71,"context_line":"        set_registry(registry)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def delete_operation(operation_id):"},{"line_number":75,"context_line":"    \"\"\""},{"line_number":76,"context_line":"    Removes the operation from the registry and deletes the event."},{"line_number":77,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"febde9ac_c01a5788","line":74,"range":{"start_line":74,"start_character":33,"end_line":74,"end_character":34},"updated":"2025-05-26 15:30:34.000000000","message":"We are currently never using this function, right?","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a3f8d1738e32c6ad3918e5df4d4a6bf183c0ec28","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        set_registry(registry)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def delete_operation(operation_id):"},{"line_number":75,"context_line":"    \"\"\""},{"line_number":76,"context_line":"    Removes the operation from the registry and deletes the event."},{"line_number":77,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"4aceba55_2f263039","line":74,"range":{"start_line":74,"start_character":33,"end_line":74,"end_character":34},"in_reply_to":"febde9ac_c01a5788","updated":"2025-05-26 17:52:23.000000000","message":"Going to use it in new version!!","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"76ca06597cfaab8f3ba0f8384a86ce771326f185","unresolved":true,"context_lines":[{"line_number":79,"context_line":"        registry \u003d get_registry()"},{"line_number":80,"context_line":"        if operation_id in registry:"},{"line_number":81,"context_line":"            del registry[operation_id]"},{"line_number":82,"context_line":"            set_registry(registry)"}],"source_content_type":"text/x-python","patch_set":1,"id":"a1e29a98_06b3cd64","line":82,"range":{"start_line":82,"start_character":25,"end_line":82,"end_character":33},"updated":"2025-05-26 15:30:34.000000000","message":"Same here, it would be nice to know when we try to delete an operation that does not exist.","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a3f8d1738e32c6ad3918e5df4d4a6bf183c0ec28","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        registry \u003d get_registry()"},{"line_number":80,"context_line":"        if operation_id in registry:"},{"line_number":81,"context_line":"            del registry[operation_id]"},{"line_number":82,"context_line":"            set_registry(registry)"}],"source_content_type":"text/x-python","patch_set":1,"id":"44f0049f_24b62c2e","line":82,"range":{"start_line":82,"start_character":25,"end_line":82,"end_character":33},"in_reply_to":"a1e29a98_06b3cd64","updated":"2025-05-26 17:52:23.000000000","message":"Done","commit_id":"a145bacdee8fcac0b95413500f73bab0f81959c4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d8ccb50b314b21facec39b9f8b08a6a912d554b","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def get_registry():"},{"line_number":38,"context_line":"    mc \u003d get_memcache_client()"},{"line_number":39,"context_line":"    serialized \u003d mc.get(REGISTRY_KEY)"},{"line_number":40,"context_line":"    if serialized:"},{"line_number":41,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"8744f66a_27a61043","line":38,"updated":"2025-05-27 14:45:03.000000000","message":"I think this idea of a \"running task registry\" is a good idea, but I don\u0027t think you need to (or should) use memcache for this. Also, I think maybe the word \"registry\" is too confusing for this, in the context of glance itself. Maybe just \"running task cancel_list\" or something.\n\nEither way, I think what you should do is use use something that doesn\u0027t require an external service to do this (combined with my just-proxy comment on the other file). Ideally we\u0027d do this in-memory with `Event` objects, but the fact that we need to support multiple process workers kinda shoots that.\n\nSo maybe the hash loop registers a task like this:\n```\nLOCK \u003d lockutils.external_lock(\u0027tasks\u0027)\n\ndef path_for_op(operation_id):\n    return os.path.join(DATA_DIR, \u0027tasks\u0027, str(operation_id))\n\ndef is_canceled(operation_id):\n    return os.path.exists(path_for_op(operation_id))\n\n@LOCK\ndef register_operation(operation_id):\n    # If this raises FileExistsError, something went wrong\n    fd \u003d os.open(oppath, os.O_CREAT | os.O_EXCL | os.O_WRONLY)\n    os.close(fd)\n```\nWhen a request to cancel comes in, we atomically create a signal file and then wait:\n```\n@LOCK\ndef cancel_operation(operation_id):\n    oppath \u003d path_for_op(operation_id)\n    with open(oppath, \u0027w\u0027) as f:\n        f.write(operation_id)\n    for try in range(60):\n        if is_canceled(operation_id):\n            return\n        time.sleep(0.5)\n\n    raise SomeException(\u0027500 Timeout canceling in-progress task\u0027)\n```\nMultiple delete operations from other workers can find and wait on the same event that way. The hash loop can monitor and signal as needed like this:\n```\n@LOCK\ndef signal_canceled(operation_id):\n    os.remove(path_for_op(oeperation_id))\n```\nIt\u0027s unfortunate to not be able to actually block on anything, and we could certainly get fancier and do that (block on a pipe or something) but the above is a single-host signaling mechanism that will work without dragging memcache into the mix.","commit_id":"8d2ec7a35b063057c0bb82a264cf62d98b426757"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"e29036662c7ecff5380f5e13657a62793c33b7fb","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def get_registry():"},{"line_number":38,"context_line":"    mc \u003d get_memcache_client()"},{"line_number":39,"context_line":"    serialized \u003d mc.get(REGISTRY_KEY)"},{"line_number":40,"context_line":"    if serialized:"},{"line_number":41,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"b90bf2a7_60d2f64f","line":38,"in_reply_to":"17fabd80_08c35042","updated":"2025-05-27 15:50:27.000000000","message":"No worries, I think splitting the patch sounds good!!","commit_id":"8d2ec7a35b063057c0bb82a264cf62d98b426757"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2201bc86bb190975ffc1501442833824a6bf4ea9","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def get_registry():"},{"line_number":38,"context_line":"    mc \u003d get_memcache_client()"},{"line_number":39,"context_line":"    serialized \u003d mc.get(REGISTRY_KEY)"},{"line_number":40,"context_line":"    if serialized:"},{"line_number":41,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"90509666_c15af2ef","line":38,"in_reply_to":"8744f66a_27a61043","updated":"2025-05-27 15:38:43.000000000","message":"I do not understood the use of writing operation_id to lock_file,\n\nAlso signal_canceled should be called from delete on the remote, right?","commit_id":"8d2ec7a35b063057c0bb82a264cf62d98b426757"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9c564c138f341358cd6e00a14d38cb59dcb3e632","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def get_registry():"},{"line_number":38,"context_line":"    mc \u003d get_memcache_client()"},{"line_number":39,"context_line":"    serialized \u003d mc.get(REGISTRY_KEY)"},{"line_number":40,"context_line":"    if serialized:"},{"line_number":41,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"17fabd80_08c35042","line":38,"in_reply_to":"90509666_c15af2ef","updated":"2025-05-27 15:46:35.000000000","message":"Sorry, I changed this a couple times while writing it in gerrit.\n\nThe is_canceled() should be checking that the file length is nonzero. Writing to the file serves to signal the hash loop to exit. That also means the use of is_canceled() in the waiter is wrong - it needs to wait for the file to go away.\n\nBasically, \"file exists\" means \"task is running\". The task runner should delete that file when it stops, whether canceled or not. Zero length means \"keep running\" nonzero means \"signaled to exit\". The waiter also needs to check that the file exists while it holds the lock before it writes to it, to avoid the race of checking before it writes to the file and it is deleted in the middle (i.e the race your existing code has at L92).\n\nMy apologies, coding in gerrit comments is not very robust. Perhaps we should split out the task list/cancel stuff into a separate patch and we can collab on that with unit tests until we get something that works, then base this patch on top?","commit_id":"8d2ec7a35b063057c0bb82a264cf62d98b426757"}]}