)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"36c9b3e036dfde46a7e978c17e8e29b6edc02ba0","unresolved":true,"context_lines":[{"line_number":14,"context_line":"- Upgrade from single store to multi-store, update store name in"},{"line_number":15,"context_line":"  location metadata"},{"line_number":16,"context_line":"- Single store credential rotation"},{"line_number":17,"context_line":"- Multistore credential roataion for all s3 stores"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Closes-Bug: #2127798"},{"line_number":20,"context_line":"Change-Id: Ib281d01ecf5187fc1a02917cc1016c1572afb071"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"63542288_3f6f8ef2","line":17,"range":{"start_line":17,"start_character":32,"end_line":17,"end_character":33},"updated":"2025-10-15 13:22:56.000000000","message":"s/roataion/rotation/ if you need another patchset","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"53ec48e7ef6ecb100f9b05a89f833ba40db32bc5","unresolved":false,"context_lines":[{"line_number":14,"context_line":"- Upgrade from single store to multi-store, update store name in"},{"line_number":15,"context_line":"  location metadata"},{"line_number":16,"context_line":"- Single store credential rotation"},{"line_number":17,"context_line":"- Multistore credential roataion for all s3 stores"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Closes-Bug: #2127798"},{"line_number":20,"context_line":"Change-Id: Ib281d01ecf5187fc1a02917cc1016c1572afb071"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"066afca5_df030f33","line":17,"range":{"start_line":17,"start_character":32,"end_line":17,"end_character":33},"in_reply_to":"63542288_3f6f8ef2","updated":"2025-10-15 16:42:49.000000000","message":"Acknowledged","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"30480f97ee492de65f371a868d82aa74a3019801","unresolved":true,"context_lines":[{"line_number":14,"context_line":"- Upgrade from single store to multi-store, update store name in"},{"line_number":15,"context_line":"  location metadata"},{"line_number":16,"context_line":"- Single store credential rotation"},{"line_number":17,"context_line":"- Multistore credential roataion for all s3 stores"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Closes-Bug: #2127798"},{"line_number":20,"context_line":"Change-Id: Ib281d01ecf5187fc1a02917cc1016c1572afb071"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"6eb362bc_1e49f1d1","line":17,"range":{"start_line":17,"start_character":24,"end_line":17,"end_character":32},"updated":"2025-10-24 15:24:38.000000000","message":"I just noticed this never got fixed. Please fix this if you respin.","commit_id":"9bb7ffd4eda728fe74de2baf7767c5f655916ad4"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f90ecad0_924220aa","updated":"2025-10-14 18:38:22.000000000","message":"Thank you for inputs dan, pushing new PS with changes we discussed. Kindly review it when you have time.","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3ef9c0532fb70b5bc04f30f424c9d17b9d5fa85d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"9edb2da8_ff4f6cbd","updated":"2025-10-20 16:19:49.000000000","message":"Hi Dan,\n\nplease find the inline response. thank you for review.","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"36c9b3e036dfde46a7e978c17e8e29b6edc02ba0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"4a1ddf82_0f6c9c1c","updated":"2025-10-15 13:22:56.000000000","message":"Just one question inline but overall looks good to me","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"10ecddd75aee4ad9dcead0b709e355119f803958","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3e5eafe8_33eece08","updated":"2025-10-15 19:13:56.000000000","message":"Thank you!","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f361e71349ef1b31df8164fec02d48b8b52ab364","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"78f86e43_66de8a65","updated":"2025-10-22 04:58:11.000000000","message":"Thank you for the input, made changes as per your suggestion.","commit_id":"31691afc005d6a7060c31caacd9a879e519a3b6d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4c483c7517057dce52197259505b0445735a86c3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"3108bf01_de7a2241","updated":"2025-10-23 21:12:32.000000000","message":"Hi Dan, \nThank you for inputs, I have made changes as per suggestion.","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4df30dc156cc3b094b3f5b106d891aea7f072b29","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"0ead4fbe_f95e7f9c","updated":"2025-10-23 15:55:49.000000000","message":"I think there\u0027s a bug here - the test that asserts no update is not strict enough and the coverage report seems to indicate that we\u0027re always updating even when we aren\u0027t meant to...","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0c5ef5a76fa3f2b54293c666919523e02c58bceb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ad4a6275_50022093","in_reply_to":"0ead4fbe_f95e7f9c","updated":"2025-10-23 20:13:31.000000000","message":"I pulled this down and did some playing with it.. I think it\u0027s just a missing test case and not a test that is incorrectly passing...","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"93fe4156134bd93b9ac45033f07f17513fff07d1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"0351a17e_1a5cb2b7","updated":"2025-10-24 14:07:17.000000000","message":"One question about the logging, but I won\u0027t hold it up unless it triggers you to think it needs changing.","commit_id":"f0649c15266f206611f5709cb920afbb3f588534"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"35dad54c9b54aa2085b7b07ab73c5766903b94e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"68696c82_5d7c6382","updated":"2025-10-27 20:08:18.000000000","message":"recheck same error as last related to boot encrypted volume","commit_id":"9bb7ffd4eda728fe74de2baf7767c5f655916ad4"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f2686a627f8cde643e017e41be5399dfb5794d4a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"463a6d64_304c15fe","updated":"2025-10-27 17:57:38.000000000","message":"recheck unrelated volume boot related error","commit_id":"9bb7ffd4eda728fe74de2baf7767c5f655916ad4"}],"glance/common/store_utils.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a03c61c9eb080a2f71b6c0b6e9bc3f59b58723a3","unresolved":true,"context_lines":[{"line_number":191,"context_line":"        # Look for credentials in the netloc part"},{"line_number":192,"context_line":"        if \u0027@\u0027 in parsed.netloc:"},{"line_number":193,"context_line":"            creds_part \u003d parsed.netloc.split(\u0027@\u0027)[0]"},{"line_number":194,"context_line":"            if \u0027:\u0027 in creds_part:"},{"line_number":195,"context_line":"                access_key, secret_key \u003d creds_part.split(\u0027:\u0027, 1)"},{"line_number":196,"context_line":"                return True, access_key, secret_key"},{"line_number":197,"context_line":"        return False, None, None"}],"source_content_type":"text/x-python","patch_set":1,"id":"c5f0097f_cefc9587","line":194,"updated":"2025-10-14 14:23:03.000000000","message":"I think there\u0027s a missing test for when there are no creds...","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[{"line_number":191,"context_line":"        # Look for credentials in the netloc part"},{"line_number":192,"context_line":"        if \u0027@\u0027 in parsed.netloc:"},{"line_number":193,"context_line":"            creds_part \u003d parsed.netloc.split(\u0027@\u0027)[0]"},{"line_number":194,"context_line":"            if \u0027:\u0027 in creds_part:"},{"line_number":195,"context_line":"                access_key, secret_key \u003d creds_part.split(\u0027:\u0027, 1)"},{"line_number":196,"context_line":"                return True, access_key, secret_key"},{"line_number":197,"context_line":"        return False, None, None"}],"source_content_type":"text/x-python","patch_set":1,"id":"607f8d70_47fee649","line":194,"in_reply_to":"c5f0097f_cefc9587","updated":"2025-10-14 18:38:22.000000000","message":"Acknowledged","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a03c61c9eb080a2f71b6c0b6e9bc3f59b58723a3","unresolved":true,"context_lines":[{"line_number":193,"context_line":"            creds_part \u003d parsed.netloc.split(\u0027@\u0027)[0]"},{"line_number":194,"context_line":"            if \u0027:\u0027 in creds_part:"},{"line_number":195,"context_line":"                access_key, secret_key \u003d creds_part.split(\u0027:\u0027, 1)"},{"line_number":196,"context_line":"                return True, access_key, secret_key"},{"line_number":197,"context_line":"        return False, None, None"},{"line_number":198,"context_line":"    except Exception as e:"},{"line_number":199,"context_line":"        LOG.debug(\"Failed to parse S3 URI %s: %s\", uri, str(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ee2aa94_0b848a1d","line":196,"updated":"2025-10-14 14:23:03.000000000","message":"You don\u0027t need to do this. `parsed.username` and `parsed.password` will have the `access_key` and `secret_key` in them.\n\nAlso, not sure we need the `is_valid` thing.. if the username and password are either/both None, we know it\u0027s not valid right?","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[{"line_number":193,"context_line":"            creds_part \u003d parsed.netloc.split(\u0027@\u0027)[0]"},{"line_number":194,"context_line":"            if \u0027:\u0027 in creds_part:"},{"line_number":195,"context_line":"                access_key, secret_key \u003d creds_part.split(\u0027:\u0027, 1)"},{"line_number":196,"context_line":"                return True, access_key, secret_key"},{"line_number":197,"context_line":"        return False, None, None"},{"line_number":198,"context_line":"    except Exception as e:"},{"line_number":199,"context_line":"        LOG.debug(\"Failed to parse S3 URI %s: %s\", uri, str(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"b06377f9_abf2002b","line":196,"in_reply_to":"1ee2aa94_0b848a1d","updated":"2025-10-14 18:38:22.000000000","message":"Makes sense","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a03c61c9eb080a2f71b6c0b6e9bc3f59b58723a3","unresolved":true,"context_lines":[{"line_number":195,"context_line":"                access_key, secret_key \u003d creds_part.split(\u0027:\u0027, 1)"},{"line_number":196,"context_line":"                return True, access_key, secret_key"},{"line_number":197,"context_line":"        return False, None, None"},{"line_number":198,"context_line":"    except Exception as e:"},{"line_number":199,"context_line":"        LOG.debug(\"Failed to parse S3 URI %s: %s\", uri, str(e))"},{"line_number":200,"context_line":"        return False, None, None"},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"f2440b82_5422a916","line":198,"updated":"2025-10-14 14:23:03.000000000","message":"This seems overly cautious, especially since we\u0027re already running this inside a `try..except Exception` from the caller. This is also small enough that I think we\u0027re pretty safe, right?\n\nAlways running everything catching Exception can make truly unexpected errors hard to debug. What are you specifically concerned about here? Also, this code is never executed in the tests...","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                access_key, secret_key \u003d creds_part.split(\u0027:\u0027, 1)"},{"line_number":196,"context_line":"                return True, access_key, secret_key"},{"line_number":197,"context_line":"        return False, None, None"},{"line_number":198,"context_line":"    except Exception as e:"},{"line_number":199,"context_line":"        LOG.debug(\"Failed to parse S3 URI %s: %s\", uri, str(e))"},{"line_number":200,"context_line":"        return False, None, None"},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5e983dd6_20deba94","line":198,"in_reply_to":"f2440b82_5422a916","updated":"2025-10-14 18:38:22.000000000","message":"Just errors from urlparse.urlparse(uri) call,  but yes, we have outer exception block where it will caught anyway.","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a03c61c9eb080a2f71b6c0b6e9bc3f59b58723a3","unresolved":true,"context_lines":[{"line_number":215,"context_line":"        if not parsed.scheme.startswith(\u0027s3\u0027):"},{"line_number":216,"context_line":"            return False"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        # Build new netloc with credentials"},{"line_number":219,"context_line":"        if \u0027@\u0027 in parsed.netloc:"},{"line_number":220,"context_line":"            # Replace existing credentials"},{"line_number":221,"context_line":"            host_part \u003d parsed.netloc.split(\u0027@\u0027)[1]"},{"line_number":222,"context_line":"            new_netloc \u003d \"%s:%s@%s\" % ("},{"line_number":223,"context_line":"                new_access_key, new_secret_key, host_part)"},{"line_number":224,"context_line":"        else:"},{"line_number":225,"context_line":"            # Add credentials to netloc"},{"line_number":226,"context_line":"            new_netloc \u003d \"%s:%s@%s\" % ("},{"line_number":227,"context_line":"                new_access_key, new_secret_key, parsed.netloc)"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"        # Rebuild the URI"},{"line_number":230,"context_line":"        new_uri \u003d urlparse.urlunparse(("}],"source_content_type":"text/x-python","patch_set":1,"id":"2af8393e_e1713531","line":227,"range":{"start_line":218,"start_character":0,"end_line":227,"end_character":62},"updated":"2025-10-14 14:23:03.000000000","message":"All of this could be simplified to:\n\n```suggestion\n        # Build new netloc with credentials\n        host_part \u003d parsed.netloc.split(\u0027@\u0027)[-1]\n        new_netloc \u003d \"%s:%s@%s\" % (\n            new_access_key, new_secret_key, host_part)\n```","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[{"line_number":215,"context_line":"        if not parsed.scheme.startswith(\u0027s3\u0027):"},{"line_number":216,"context_line":"            return False"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        # Build new netloc with credentials"},{"line_number":219,"context_line":"        if \u0027@\u0027 in parsed.netloc:"},{"line_number":220,"context_line":"            # Replace existing credentials"},{"line_number":221,"context_line":"            host_part \u003d parsed.netloc.split(\u0027@\u0027)[1]"},{"line_number":222,"context_line":"            new_netloc \u003d \"%s:%s@%s\" % ("},{"line_number":223,"context_line":"                new_access_key, new_secret_key, host_part)"},{"line_number":224,"context_line":"        else:"},{"line_number":225,"context_line":"            # Add credentials to netloc"},{"line_number":226,"context_line":"            new_netloc \u003d \"%s:%s@%s\" % ("},{"line_number":227,"context_line":"                new_access_key, new_secret_key, parsed.netloc)"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"        # Rebuild the URI"},{"line_number":230,"context_line":"        new_uri \u003d urlparse.urlunparse(("}],"source_content_type":"text/x-python","patch_set":1,"id":"40f86c5c_1e770dc4","line":227,"range":{"start_line":218,"start_character":0,"end_line":227,"end_character":62},"in_reply_to":"2af8393e_e1713531","updated":"2025-10-14 18:38:22.000000000","message":"Acknowledged","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a03c61c9eb080a2f71b6c0b6e9bc3f59b58723a3","unresolved":true,"context_lines":[{"line_number":240,"context_line":"        LOG.debug(\"Updated S3 location URI with new credentials\")"},{"line_number":241,"context_line":"        return True"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    except Exception as e:"},{"line_number":244,"context_line":"        LOG.warning(\"Failed to update S3 location with new credentials: %s\","},{"line_number":245,"context_line":"                    str(e))"},{"line_number":246,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"d6c37eeb_f09b621f","line":243,"updated":"2025-10-14 14:23:03.000000000","message":"This too.","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        LOG.debug(\"Updated S3 location URI with new credentials\")"},{"line_number":241,"context_line":"        return True"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    except Exception as e:"},{"line_number":244,"context_line":"        LOG.warning(\"Failed to update S3 location with new credentials: %s\","},{"line_number":245,"context_line":"                    str(e))"},{"line_number":246,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"cec817ec_4f5ea5b7","line":243,"in_reply_to":"d6c37eeb_f09b621f","updated":"2025-10-14 18:38:22.000000000","message":"Acknowledged","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a03c61c9eb080a2f71b6c0b6e9bc3f59b58723a3","unresolved":true,"context_lines":[{"line_number":272,"context_line":""},{"line_number":273,"context_line":"        # Handle S3 locations that might need credential updates"},{"line_number":274,"context_line":"        if loc[\u0027url\u0027].startswith((\u0027s3://\u0027, \u0027s3+http://\u0027, \u0027s3+https://\u0027)):"},{"line_number":275,"context_line":"            if _update_s3_credentials_if_needed("},{"line_number":276,"context_line":"                    context, loc, image, image_repo):"},{"line_number":277,"context_line":"                store_updated \u003d True"},{"line_number":278,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"63aab3df_ff48e0a1","line":275,"updated":"2025-10-14 14:23:03.000000000","message":"The coverage report is saying this is _always_ returning true, at least in the test run. I haven\u0027t debugged it, but perhaps one of the comparisons in the \"if needed\" function is providing a false need to always update?","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[{"line_number":272,"context_line":""},{"line_number":273,"context_line":"        # Handle S3 locations that might need credential updates"},{"line_number":274,"context_line":"        if loc[\u0027url\u0027].startswith((\u0027s3://\u0027, \u0027s3+http://\u0027, \u0027s3+https://\u0027)):"},{"line_number":275,"context_line":"            if _update_s3_credentials_if_needed("},{"line_number":276,"context_line":"                    context, loc, image, image_repo):"},{"line_number":277,"context_line":"                store_updated \u003d True"},{"line_number":278,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"f95a0457_9ac35ba3","line":275,"in_reply_to":"63aab3df_ff48e0a1","updated":"2025-10-14 18:38:22.000000000","message":"Good catch, will correct it.","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a03c61c9eb080a2f71b6c0b6e9bc3f59b58723a3","unresolved":true,"context_lines":[{"line_number":309,"context_line":"        access_key \u003d getattr(store_instance, \u0027access_key\u0027, None)"},{"line_number":310,"context_line":"        secret_key \u003d getattr(store_instance, \u0027secret_key\u0027, None)"},{"line_number":311,"context_line":"        if not access_key or not secret_key:"},{"line_number":312,"context_line":"            return False"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        # Check if credentials in URI need updating"},{"line_number":315,"context_line":"        is_valid, uri_access_key, uri_secret_key \u003d _validate_s3_credentials("}],"source_content_type":"text/x-python","patch_set":1,"id":"04faeebd_eb0766b5","line":312,"updated":"2025-10-14 14:23:03.000000000","message":"This is not covered in the tests","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[{"line_number":309,"context_line":"        access_key \u003d getattr(store_instance, \u0027access_key\u0027, None)"},{"line_number":310,"context_line":"        secret_key \u003d getattr(store_instance, \u0027secret_key\u0027, None)"},{"line_number":311,"context_line":"        if not access_key or not secret_key:"},{"line_number":312,"context_line":"            return False"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        # Check if credentials in URI need updating"},{"line_number":315,"context_line":"        is_valid, uri_access_key, uri_secret_key \u003d _validate_s3_credentials("}],"source_content_type":"text/x-python","patch_set":1,"id":"13121b8b_8e0d2207","line":312,"in_reply_to":"04faeebd_eb0766b5","updated":"2025-10-14 18:38:22.000000000","message":"Acknowledged","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a03c61c9eb080a2f71b6c0b6e9bc3f59b58723a3","unresolved":true,"context_lines":[{"line_number":315,"context_line":"        is_valid, uri_access_key, uri_secret_key \u003d _validate_s3_credentials("},{"line_number":316,"context_line":"            loc[\u0027url\u0027])"},{"line_number":317,"context_line":"        if not is_valid:"},{"line_number":318,"context_line":"            return False"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        needs_update \u003d False"},{"line_number":321,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"25965870_8559785e","line":318,"updated":"2025-10-14 14:23:03.000000000","message":"Nor this.","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[{"line_number":315,"context_line":"        is_valid, uri_access_key, uri_secret_key \u003d _validate_s3_credentials("},{"line_number":316,"context_line":"            loc[\u0027url\u0027])"},{"line_number":317,"context_line":"        if not is_valid:"},{"line_number":318,"context_line":"            return False"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        needs_update \u003d False"},{"line_number":321,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"b4383adb_7de01df4","line":318,"in_reply_to":"25965870_8559785e","updated":"2025-10-14 18:38:22.000000000","message":"Acknowledged","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a03c61c9eb080a2f71b6c0b6e9bc3f59b58723a3","unresolved":true,"context_lines":[{"line_number":323,"context_line":"        if uri_access_key !\u003d access_key or uri_secret_key !\u003d secret_key:"},{"line_number":324,"context_line":"            if not _update_s3_location_with_new_credentials("},{"line_number":325,"context_line":"                    loc, access_key, secret_key):"},{"line_number":326,"context_line":"                return False"},{"line_number":327,"context_line":"            LOG.info(\"Updated S3 credentials for image %s\", image.image_id)"},{"line_number":328,"context_line":"            needs_update \u003d True"},{"line_number":329,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"e76f1e75_31775ae8","line":326,"updated":"2025-10-14 14:23:03.000000000","message":"Nor this.","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        if uri_access_key !\u003d access_key or uri_secret_key !\u003d secret_key:"},{"line_number":324,"context_line":"            if not _update_s3_location_with_new_credentials("},{"line_number":325,"context_line":"                    loc, access_key, secret_key):"},{"line_number":326,"context_line":"                return False"},{"line_number":327,"context_line":"            LOG.info(\"Updated S3 credentials for image %s\", image.image_id)"},{"line_number":328,"context_line":"            needs_update \u003d True"},{"line_number":329,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"35dc2c38_631ab5ab","line":326,"in_reply_to":"e76f1e75_31775ae8","updated":"2025-10-14 18:38:22.000000000","message":"Acknowledged","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a03c61c9eb080a2f71b6c0b6e9bc3f59b58723a3","unresolved":true,"context_lines":[{"line_number":338,"context_line":""},{"line_number":339,"context_line":"    except Exception as e:"},{"line_number":340,"context_line":"        LOG.warning(\"Error updating S3 credentials for image %s: %s\","},{"line_number":341,"context_line":"                    image.image_id, str(e))"},{"line_number":342,"context_line":"        return False"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"8ace03cb_07e6b933","line":341,"updated":"2025-10-14 14:23:03.000000000","message":"This should be covering all the above cases, so if we really need it, then this should be the only one required I think. Either way, I think it would be best to `LOG.exception()` here as truly unexpected exceptions we can\u0027t even predict ahead of time will surely need a traceback to know what is failing right?","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[{"line_number":338,"context_line":""},{"line_number":339,"context_line":"    except Exception as e:"},{"line_number":340,"context_line":"        LOG.warning(\"Error updating S3 credentials for image %s: %s\","},{"line_number":341,"context_line":"                    image.image_id, str(e))"},{"line_number":342,"context_line":"        return False"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"918abcde_1d7521f1","line":341,"in_reply_to":"8ace03cb_07e6b933","updated":"2025-10-14 18:38:22.000000000","message":"+1","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a03c61c9eb080a2f71b6c0b6e9bc3f59b58723a3","unresolved":true,"context_lines":[{"line_number":354,"context_line":"            scheme_map \u003d store_api.location.SCHEME_TO_CLS_BACKEND_MAP[scheme]"},{"line_number":355,"context_line":"            if store_name in scheme_map:"},{"line_number":356,"context_line":"                return scheme_map[store_name][\u0027store\u0027]"},{"line_number":357,"context_line":"    return None"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"def _update_cinder_location_and_store_id(context, loc):"}],"source_content_type":"text/x-python","patch_set":1,"id":"a6f1d780_958792c6","line":357,"updated":"2025-10-14 14:23:03.000000000","message":"`return None` is, of course, implied :)","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4d5e71d52e3799399e08d5dc8d758795c62bc2d4","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            scheme_map \u003d store_api.location.SCHEME_TO_CLS_BACKEND_MAP[scheme]"},{"line_number":355,"context_line":"            if store_name in scheme_map:"},{"line_number":356,"context_line":"                return scheme_map[store_name][\u0027store\u0027]"},{"line_number":357,"context_line":"    return None"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"def _update_cinder_location_and_store_id(context, loc):"}],"source_content_type":"text/x-python","patch_set":1,"id":"8053cb30_fa46d24e","line":357,"in_reply_to":"a6f1d780_958792c6","updated":"2025-10-14 18:38:22.000000000","message":"Right!","commit_id":"9a5a8b3eb7df0159bd50f09be0dd91298e7e65c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a7d708bc2b84512de8ad3c014c186ce98cba2077","unresolved":true,"context_lines":[{"line_number":305,"context_line":"            if (access_key \u003d\u003d new_access_key and"},{"line_number":306,"context_line":"                    secret_key \u003d\u003d new_secret_key):"},{"line_number":307,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":308,"context_line":"                return True"},{"line_number":309,"context_line":"        return False"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"970b8b1d_e75a2224","line":308,"updated":"2025-10-20 15:53:06.000000000","message":"I don\u0027t understand this.. if we don\u0027t have a store name, we iterate all the stores until we find one that...already matches the new key/secret, update the store name and return \"something changed\"? Isn\u0027t the point of this \"case 2\" to update stores that needs new credentials but don\u0027t have a store name set?\n\nIf you\u0027re really just handling the case where we\u0027re migrating from no store name to one with -- shouldn\u0027t we be checking more here? Like, we should be expecting the entire URL to be the same, not just the credentials, no? Couldn\u0027t we potentially have two stores with the same credentials but different actual URLs?\n\nIf someone needs to update creds, and has images that are still store-name-less, they will have to do what? Update config, and GET every image in the deployment to update the store names, then change config for the new creds and re-GET them all?","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"124342a6cec41e278581bd1a4cf36bf3b4cba5f4","unresolved":true,"context_lines":[{"line_number":305,"context_line":"            if (access_key \u003d\u003d new_access_key and"},{"line_number":306,"context_line":"                    secret_key \u003d\u003d new_secret_key):"},{"line_number":307,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":308,"context_line":"                return True"},{"line_number":309,"context_line":"        return False"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"b7b7e587_c0ad8fd1","line":308,"in_reply_to":"104ef757_901bf820","updated":"2025-10-20 19:45:55.000000000","message":"...I realize that, but then you went on to talk about needing to compare the bucket id which seemed to imply you were going to make a change because it _is_ too risky. To me, it seems very risky, assuming I do the following:\n\n1. Have all my glances configured with a single s3 store\n2. I want to enable a second store (same credentials, different bucket) so I roll out a new multistore config and restart glance\n3. As soon as an image is GET\u0027d, it may be decorated with the wrong/new store since the credentials are the same\n\nam I wrong?","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7321535d7a54671382ea2543cc9e3295f5664940","unresolved":false,"context_lines":[{"line_number":305,"context_line":"            if (access_key \u003d\u003d new_access_key and"},{"line_number":306,"context_line":"                    secret_key \u003d\u003d new_secret_key):"},{"line_number":307,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":308,"context_line":"                return True"},{"line_number":309,"context_line":"        return False"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5c730136_7cb24e8e","line":308,"in_reply_to":"1db89cf8_991cac93","updated":"2025-10-21 06:51:56.000000000","message":"Done","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"206bdf6d4f1ef3ac3560bb4855c2682a571381af","unresolved":true,"context_lines":[{"line_number":305,"context_line":"            if (access_key \u003d\u003d new_access_key and"},{"line_number":306,"context_line":"                    secret_key \u003d\u003d new_secret_key):"},{"line_number":307,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":308,"context_line":"                return True"},{"line_number":309,"context_line":"        return False"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"c5062e7d_d524c08d","line":308,"in_reply_to":"89e58c4f_3023473a","updated":"2025-10-20 16:23:31.000000000","message":"But what if I\u0027m configured for single store now, and once I enable multi-store, I have two s3 stores with the same credentials configured? Isn\u0027t this assumption too risky?","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3ef9c0532fb70b5bc04f30f424c9d17b9d5fa85d","unresolved":true,"context_lines":[{"line_number":305,"context_line":"            if (access_key \u003d\u003d new_access_key and"},{"line_number":306,"context_line":"                    secret_key \u003d\u003d new_secret_key):"},{"line_number":307,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":308,"context_line":"                return True"},{"line_number":309,"context_line":"        return False"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"89e58c4f_3023473a","line":308,"in_reply_to":"970b8b1d_e75a2224","updated":"2025-10-20 16:19:49.000000000","message":"Case 2 is the case where we are upgrading from single store (traditional way using stores param in config to use enabled_backends). This case here there is just upgrade and no credential rotation. Here we just need to add the store name in the metadata.\n\nIn upgrade case there is no way where we can have more than one s3 store, so no use of checking entire URL imo. \n\ni am not sure for the possibility of 3rd case where user wants to upgrade from single store to multi store where he also want to change the credentials as well and that is why I didn’t considered that scenario here.","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"5ae52de0eb38d19bf2f71399b4f511510eeb9e71","unresolved":true,"context_lines":[{"line_number":305,"context_line":"            if (access_key \u003d\u003d new_access_key and"},{"line_number":306,"context_line":"                    secret_key \u003d\u003d new_secret_key):"},{"line_number":307,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":308,"context_line":"                return True"},{"line_number":309,"context_line":"        return False"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1db89cf8_991cac93","line":308,"in_reply_to":"b7b7e587_c0ad8fd1","updated":"2025-10-20 19:50:55.000000000","message":"No you are not wrong, I agreed to your earlier comment. May be earlier i failed to reply you correctly. I will fix the code to include this scenario.","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8647224311f2a4ebe7a72f34c74ffc7cab664518","unresolved":true,"context_lines":[{"line_number":305,"context_line":"            if (access_key \u003d\u003d new_access_key and"},{"line_number":306,"context_line":"                    secret_key \u003d\u003d new_secret_key):"},{"line_number":307,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":308,"context_line":"                return True"},{"line_number":309,"context_line":"        return False"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"104ef757_901bf820","line":308,"in_reply_to":"bc4a53bd_ac7d6b8f","updated":"2025-10-20 19:12:54.000000000","message":"i was saying No to your last question “isn’t this assumption too risky?”","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"c5defebd7a45b3bf8a1cfe5f6613c89a40496ed2","unresolved":true,"context_lines":[{"line_number":305,"context_line":"            if (access_key \u003d\u003d new_access_key and"},{"line_number":306,"context_line":"                    secret_key \u003d\u003d new_secret_key):"},{"line_number":307,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":308,"context_line":"                return True"},{"line_number":309,"context_line":"        return False"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"e5a1d56a_47ec1d98","line":308,"in_reply_to":"c5062e7d_d524c08d","updated":"2025-10-20 17:58:27.000000000","message":"No, that’s not risky. i think in this case i need to consider bucket option as well. I will cover that case as well if we consider that bucket name should be unique and not allow to change once set.","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"536920a502fddf10dd89f6514e84993a128863ab","unresolved":true,"context_lines":[{"line_number":305,"context_line":"            if (access_key \u003d\u003d new_access_key and"},{"line_number":306,"context_line":"                    secret_key \u003d\u003d new_secret_key):"},{"line_number":307,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":308,"context_line":"                return True"},{"line_number":309,"context_line":"        return False"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bc4a53bd_ac7d6b8f","line":308,"in_reply_to":"e5a1d56a_47ec1d98","updated":"2025-10-20 18:21:41.000000000","message":"\u003e No, that’s not risky.\n\nIt\u0027s not? Seems like the rest of your answer implies needing a change...\n\n\u003e i think in this case i need to consider bucket option as well. I will cover that case as well if we consider that bucket name should be unique and not allow to change once set.\n\nRight, bucket name as the unique identifier in the absence of a name seems like the correct behavior to me.","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5dc298223b3d7f284ba6d23520365f24d4f64258","unresolved":true,"context_lines":[{"line_number":316,"context_line":"            # Construct expected entire URL for this store"},{"line_number":317,"context_line":"            expected_url \u003d _construct_s3_url("},{"line_number":318,"context_line":"                store_instance, scheme, parsed.path)"},{"line_number":319,"context_line":"            if expected_url and loc[\u0027url\u0027] \u003d\u003d expected_url:"},{"line_number":320,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":321,"context_line":"                return True"},{"line_number":322,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":6,"id":"69ab15af_b459dc43","line":319,"updated":"2025-10-21 14:01:36.000000000","message":"This is better for sure (and I prefer the whole-URL comparison for case 1) but.. I thought you were going to deconstruct it and look at just the bucket? If I have to do a credential rotation and move to multi-store at or around the same time, this will not work. Meaning, if I do:\n\n1. Deploy new multi-store config with different credentials for existing store\n2. GET images will not fail, but also not update store name\n3. Remove old credentials because I think I\u0027ve rotated everything\n4. All those GETs will now fail because they weren\u0027t updated and won\u0027t get new credentials\n\nright? What I\u0027d have to do is this more complex process:\n\n1. Deploy new multi-store config with *same* credentials for existing store\n2. GET every existing image in my deployment to update store\n3. Re-deploy with updated credentials\n4. Re-GET every existing image in my deployment to update creds\n5. Remove old credentials\n\nIf, for example, on L294 we did something like:\n```\nstore_name \u003d loc[\u0027metadata\u0027].get(\u0027store\u0027)\nif store_name:\n    # Multistore, find by store name\n    store_instance \u003d location_map[scheme][store_name][\u0027store\u0027]\nelse:\n    # Old single store instance. Find by bucket and update store name for next time\n    store_instance \u003d find_store_by_bucket()\n    loc[\u0027metadata\u0027][\u0027store\u0027] \u003d existing[\u0027store\u0027]\n\n# For any store (old or new), update creds\nexpected_url \u003d _construct_s3_url(store_instance, scheme, parsed.path)\nif expected_url and loc[\u0027url\u0027] !\u003d expected_url:\n    LOG.debug(\"S3 URL mismatch, updating URL\")\n    new_access_key, new_secret_key \u003d _get_store_credentials(\n        store_instance)\n    loc[\u0027url\u0027] \u003d _update_s3_url(\n        parsed, new_access_key, new_secret_key)\n```\n\nthat would always work in one step, no?","commit_id":"31691afc005d6a7060c31caacd9a879e519a3b6d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f361e71349ef1b31df8164fec02d48b8b52ab364","unresolved":false,"context_lines":[{"line_number":316,"context_line":"            # Construct expected entire URL for this store"},{"line_number":317,"context_line":"            expected_url \u003d _construct_s3_url("},{"line_number":318,"context_line":"                store_instance, scheme, parsed.path)"},{"line_number":319,"context_line":"            if expected_url and loc[\u0027url\u0027] \u003d\u003d expected_url:"},{"line_number":320,"context_line":"                loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":321,"context_line":"                return True"},{"line_number":322,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":6,"id":"445616bd_5eec3cf9","line":319,"in_reply_to":"69ab15af_b459dc43","updated":"2025-10-22 04:58:11.000000000","message":"Yeah, this looks better","commit_id":"31691afc005d6a7060c31caacd9a879e519a3b6d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4df30dc156cc3b094b3f5b106d891aea7f072b29","unresolved":true,"context_lines":[{"line_number":273,"context_line":"    for store_name, store_info in location_map[scheme].items():"},{"line_number":274,"context_line":"        store_instance \u003d store_info[\u0027store\u0027]"},{"line_number":275,"context_line":"        if store_instance.bucket \u003d\u003d bucket_name:"},{"line_number":276,"context_line":"            return (store_name, store_instance)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"def _construct_s3_url(store_instance, scheme, path):"}],"source_content_type":"text/x-python","patch_set":7,"id":"319439f3_6437fe24","line":276,"updated":"2025-10-23 15:55:49.000000000","message":"Hmm, do we need to return this tuple? Doesn\u0027t the store instance have the name on it? Or is it just available in the map?","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4c483c7517057dce52197259505b0445735a86c3","unresolved":false,"context_lines":[{"line_number":273,"context_line":"    for store_name, store_info in location_map[scheme].items():"},{"line_number":274,"context_line":"        store_instance \u003d store_info[\u0027store\u0027]"},{"line_number":275,"context_line":"        if store_instance.bucket \u003d\u003d bucket_name:"},{"line_number":276,"context_line":"            return (store_name, store_instance)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"def _construct_s3_url(store_instance, scheme, path):"}],"source_content_type":"text/x-python","patch_set":7,"id":"0164aa1d_38789fbc","line":276,"in_reply_to":"319439f3_6437fe24","updated":"2025-10-23 21:12:32.000000000","message":"It exists only in the map structure, not as an attribute of the store instance.","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4df30dc156cc3b094b3f5b106d891aea7f072b29","unresolved":true,"context_lines":[{"line_number":297,"context_line":"    parsed \u003d urlparse.urlparse(uri)"},{"line_number":298,"context_line":"    scheme \u003d parsed.scheme"},{"line_number":299,"context_line":"    location_map \u003d store_api.location.SCHEME_TO_CLS_BACKEND_MAP"},{"line_number":300,"context_line":"    if scheme not in location_map:"},{"line_number":301,"context_line":"        LOG.debug(\"Unknown scheme \u0027%(scheme)s\u0027 found in uri \u0027%(uri)s\u0027\","},{"line_number":302,"context_line":"                  {\u0027scheme\u0027: scheme, \u0027uri\u0027: uri})"},{"line_number":303,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":7,"id":"eb2ae19b_50ae4ab6","line":300,"updated":"2025-10-23 15:55:49.000000000","message":"This is missing a test case","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7bee95e1ab4500a8bf0cbf96194fe269735444d2","unresolved":false,"context_lines":[{"line_number":297,"context_line":"    parsed \u003d urlparse.urlparse(uri)"},{"line_number":298,"context_line":"    scheme \u003d parsed.scheme"},{"line_number":299,"context_line":"    location_map \u003d store_api.location.SCHEME_TO_CLS_BACKEND_MAP"},{"line_number":300,"context_line":"    if scheme not in location_map:"},{"line_number":301,"context_line":"        LOG.debug(\"Unknown scheme \u0027%(scheme)s\u0027 found in uri \u0027%(uri)s\u0027\","},{"line_number":302,"context_line":"                  {\u0027scheme\u0027: scheme, \u0027uri\u0027: uri})"},{"line_number":303,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":7,"id":"ecc92ba9_5fbe79f1","line":300,"in_reply_to":"6d337179_1a44236b","updated":"2025-10-24 14:13:46.000000000","message":"+1","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"93fe4156134bd93b9ac45033f07f17513fff07d1","unresolved":false,"context_lines":[{"line_number":297,"context_line":"    parsed \u003d urlparse.urlparse(uri)"},{"line_number":298,"context_line":"    scheme \u003d parsed.scheme"},{"line_number":299,"context_line":"    location_map \u003d store_api.location.SCHEME_TO_CLS_BACKEND_MAP"},{"line_number":300,"context_line":"    if scheme not in location_map:"},{"line_number":301,"context_line":"        LOG.debug(\"Unknown scheme \u0027%(scheme)s\u0027 found in uri \u0027%(uri)s\u0027\","},{"line_number":302,"context_line":"                  {\u0027scheme\u0027: scheme, \u0027uri\u0027: uri})"},{"line_number":303,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":7,"id":"6d337179_1a44236b","line":300,"in_reply_to":"a4bad734_21d1f5a2","updated":"2025-10-24 14:07:17.000000000","message":"_Intended_ to be covered you mean :) I was referring to the code coverage report which showed that it definitely wasn\u0027t, but I see the change you made to the test to fix it properly, so that\u0027s good.","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4c483c7517057dce52197259505b0445735a86c3","unresolved":false,"context_lines":[{"line_number":297,"context_line":"    parsed \u003d urlparse.urlparse(uri)"},{"line_number":298,"context_line":"    scheme \u003d parsed.scheme"},{"line_number":299,"context_line":"    location_map \u003d store_api.location.SCHEME_TO_CLS_BACKEND_MAP"},{"line_number":300,"context_line":"    if scheme not in location_map:"},{"line_number":301,"context_line":"        LOG.debug(\"Unknown scheme \u0027%(scheme)s\u0027 found in uri \u0027%(uri)s\u0027\","},{"line_number":302,"context_line":"                  {\u0027scheme\u0027: scheme, \u0027uri\u0027: uri})"},{"line_number":303,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":7,"id":"a4bad734_21d1f5a2","line":300,"in_reply_to":"eb2ae19b_50ae4ab6","updated":"2025-10-23 21:12:32.000000000","message":"This is covered in test test_update_s3_credentials_unknown_scheme,  i have added log statement for additional verification.","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4df30dc156cc3b094b3f5b106d891aea7f072b29","unresolved":true,"context_lines":[{"line_number":315,"context_line":"            loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":316,"context_line":"        else:"},{"line_number":317,"context_line":"            # No matching store found"},{"line_number":318,"context_line":"            return False"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    # For any store (old or new), update creds if there\u0027s a mismatch"},{"line_number":321,"context_line":"    expected_url \u003d _construct_s3_url(store_instance, scheme, parsed.path)"}],"source_content_type":"text/x-python","patch_set":7,"id":"47686e43_27b85679","line":318,"updated":"2025-10-23 15:55:49.000000000","message":"This is probably worth a `LOG.warning()` at least right? this means there\u0027s a location in an image that we do not have a store definition for, correct? I\u0027m not really sure how bad that is, but it certainly seems relevant to someone who cares about credential rotation...","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4c483c7517057dce52197259505b0445735a86c3","unresolved":false,"context_lines":[{"line_number":315,"context_line":"            loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":316,"context_line":"        else:"},{"line_number":317,"context_line":"            # No matching store found"},{"line_number":318,"context_line":"            return False"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    # For any store (old or new), update creds if there\u0027s a mismatch"},{"line_number":321,"context_line":"    expected_url \u003d _construct_s3_url(store_instance, scheme, parsed.path)"}],"source_content_type":"text/x-python","patch_set":7,"id":"78c9a055_e41b7458","line":318,"in_reply_to":"47686e43_27b85679","updated":"2025-10-23 21:12:32.000000000","message":"Acknowledged","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4df30dc156cc3b094b3f5b106d891aea7f072b29","unresolved":true,"context_lines":[{"line_number":320,"context_line":"    # For any store (old or new), update creds if there\u0027s a mismatch"},{"line_number":321,"context_line":"    expected_url \u003d _construct_s3_url(store_instance, scheme, parsed.path)"},{"line_number":322,"context_line":"    if expected_url and loc[\u0027url\u0027] !\u003d expected_url:"},{"line_number":323,"context_line":"        LOG.debug(\"S3 URL mismatch, updating URL\")"},{"line_number":324,"context_line":"        new_access_key, new_secret_key \u003d _get_store_credentials("},{"line_number":325,"context_line":"            store_instance)"},{"line_number":326,"context_line":"        loc[\u0027url\u0027] \u003d _update_s3_url("}],"source_content_type":"text/x-python","patch_set":7,"id":"94413e63_bfe25c89","line":323,"updated":"2025-10-23 15:55:49.000000000","message":"Maybe should be `INFO` level, since having debug disabled would mean we don\u0027t see this and it\u0027s a pretty big deal right? `INFO` is supposed to be for unit-of-work...","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4c483c7517057dce52197259505b0445735a86c3","unresolved":false,"context_lines":[{"line_number":320,"context_line":"    # For any store (old or new), update creds if there\u0027s a mismatch"},{"line_number":321,"context_line":"    expected_url \u003d _construct_s3_url(store_instance, scheme, parsed.path)"},{"line_number":322,"context_line":"    if expected_url and loc[\u0027url\u0027] !\u003d expected_url:"},{"line_number":323,"context_line":"        LOG.debug(\"S3 URL mismatch, updating URL\")"},{"line_number":324,"context_line":"        new_access_key, new_secret_key \u003d _get_store_credentials("},{"line_number":325,"context_line":"            store_instance)"},{"line_number":326,"context_line":"        loc[\u0027url\u0027] \u003d _update_s3_url("}],"source_content_type":"text/x-python","patch_set":7,"id":"8154134e_6554677f","line":323,"in_reply_to":"94413e63_bfe25c89","updated":"2025-10-23 21:12:32.000000000","message":"Acknowledged","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4df30dc156cc3b094b3f5b106d891aea7f072b29","unresolved":true,"context_lines":[{"line_number":327,"context_line":"            parsed, new_access_key, new_secret_key)"},{"line_number":328,"context_line":"        return True"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    return False"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"def get_updated_store_location(locations, context\u003dNone):"}],"source_content_type":"text/x-python","patch_set":7,"id":"4a77549a_a7172d94","line":330,"updated":"2025-10-23 15:55:49.000000000","message":"Hmm, looks like we have no test case for where this doesn\u0027t update a store... I wonder if that means L322 is always True even though it shouldn\u0027t be?","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4c483c7517057dce52197259505b0445735a86c3","unresolved":false,"context_lines":[{"line_number":327,"context_line":"            parsed, new_access_key, new_secret_key)"},{"line_number":328,"context_line":"        return True"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    return False"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"def get_updated_store_location(locations, context\u003dNone):"}],"source_content_type":"text/x-python","patch_set":7,"id":"9c0674fa_0636bac4","line":330,"in_reply_to":"4a77549a_a7172d94","updated":"2025-10-23 21:12:32.000000000","message":"Acknowledged, added new test to cover this, by the way as usual you were right to point the issue :)","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"93fe4156134bd93b9ac45033f07f17513fff07d1","unresolved":true,"context_lines":[{"line_number":315,"context_line":"            loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":316,"context_line":"        else:"},{"line_number":317,"context_line":"            # No matching store found"},{"line_number":318,"context_line":"            LOG.warning(\"No S3 store found for location.\")"},{"line_number":319,"context_line":"            return False"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"    # For any store (old or new), update creds if there\u0027s a mismatch"}],"source_content_type":"text/x-python","patch_set":8,"id":"15775ec6_b4d22ab3","line":318,"updated":"2025-10-24 14:07:17.000000000","message":"For this and the INFO below, I assume we\u0027ll log the image id in question because of our context right? A bunch of \"No S3 store found for location\" messages with no info about what needs fixing would be frustrating.","commit_id":"f0649c15266f206611f5709cb920afbb3f588534"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7bee95e1ab4500a8bf0cbf96194fe269735444d2","unresolved":false,"context_lines":[{"line_number":315,"context_line":"            loc[\u0027metadata\u0027][\u0027store\u0027] \u003d store_name"},{"line_number":316,"context_line":"        else:"},{"line_number":317,"context_line":"            # No matching store found"},{"line_number":318,"context_line":"            LOG.warning(\"No S3 store found for location.\")"},{"line_number":319,"context_line":"            return False"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"    # For any store (old or new), update creds if there\u0027s a mismatch"}],"source_content_type":"text/x-python","patch_set":8,"id":"90b7fb5f_971626a9","line":318,"in_reply_to":"15775ec6_b4d22ab3","updated":"2025-10-24 14:13:46.000000000","message":"Ack, I think its better to add image id here, will push a new PS shortly","commit_id":"f0649c15266f206611f5709cb920afbb3f588534"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"93fe4156134bd93b9ac45033f07f17513fff07d1","unresolved":false,"context_lines":[{"line_number":321,"context_line":"    # For any store (old or new), update creds if there\u0027s a mismatch"},{"line_number":322,"context_line":"    # URL format: s3://key:secret@host/bucket/object"},{"line_number":323,"context_line":"    # Extract object path: everything after the bucket name"},{"line_number":324,"context_line":"    object_path \u003d parsed.path[parsed.path.find(\u0027/\u0027, 1):]"},{"line_number":325,"context_line":"    expected_url \u003d _construct_s3_url(store_instance, scheme, object_path)"},{"line_number":326,"context_line":"    if expected_url and loc[\u0027url\u0027] !\u003d expected_url:"},{"line_number":327,"context_line":"        LOG.info(\"S3 URL mismatch, updating URL\")"}],"source_content_type":"text/x-python","patch_set":8,"id":"a1e2f940_f5a888b1","line":324,"updated":"2025-10-24 14:07:17.000000000","message":"Aha :)","commit_id":"f0649c15266f206611f5709cb920afbb3f588534"}],"glance/location.py":[{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"36c9b3e036dfde46a7e978c17e8e29b6edc02ba0","unresolved":true,"context_lines":[{"line_number":133,"context_line":"                                parsed, new_access_key, new_secret_key)"},{"line_number":134,"context_line":"                            # Save the image immediately after update"},{"line_number":135,"context_line":"                            self.image_repo.save(image)"},{"line_number":136,"context_line":"                            break"},{"line_number":137,"context_line":"        return image"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"0e6fca98_93e75cbb","line":136,"range":{"start_line":136,"start_character":28,"end_line":136,"end_character":33},"updated":"2025-10-15 13:22:56.000000000","message":"I assume there can only be one location that starts with \"s3\" in single-store mode, hence why we break here?","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3c1cdb5340132247c4937f9b1fb0b85c45ea85d7","unresolved":true,"context_lines":[{"line_number":133,"context_line":"                                parsed, new_access_key, new_secret_key)"},{"line_number":134,"context_line":"                            # Save the image immediately after update"},{"line_number":135,"context_line":"                            self.image_repo.save(image)"},{"line_number":136,"context_line":"                            break"},{"line_number":137,"context_line":"        return image"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"27c364b6_2cff5ce7","line":136,"range":{"start_line":136,"start_character":28,"end_line":136,"end_character":33},"in_reply_to":"0e6fca98_93e75cbb","updated":"2025-10-15 13:28:00.000000000","message":"Even there is one default store we can have multiple stores defined in stores section like stores \u003d file, s3, swift etc and user can use location api to add any location pointing to these stores. So as soon as we found s3 store in location we process it and break the loop.","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"9ce4db046f8bfac599d67e3f3fa0f129b0d3a59e","unresolved":false,"context_lines":[{"line_number":133,"context_line":"                                parsed, new_access_key, new_secret_key)"},{"line_number":134,"context_line":"                            # Save the image immediately after update"},{"line_number":135,"context_line":"                            self.image_repo.save(image)"},{"line_number":136,"context_line":"                            break"},{"line_number":137,"context_line":"        return image"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"615fbfb5_87f4e558","line":136,"range":{"start_line":136,"start_character":28,"end_line":136,"end_character":33},"in_reply_to":"27c364b6_2cff5ce7","updated":"2025-10-15 18:58:38.000000000","message":"Acknowledged","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4df30dc156cc3b094b3f5b106d891aea7f072b29","unresolved":true,"context_lines":[{"line_number":133,"context_line":"                                parsed, new_access_key, new_secret_key)"},{"line_number":134,"context_line":"                            # Save the image immediately after update"},{"line_number":135,"context_line":"                            self.image_repo.save(image)"},{"line_number":136,"context_line":"                            break"},{"line_number":137,"context_line":"        return image"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"b0ee6068_6ef136e7","line":136,"updated":"2025-10-23 15:55:49.000000000","message":"This is sort of a nit, but.. might be good to put this into a helper, even if in this file so that it looks like the multistore case above and is easier to follow.","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4c483c7517057dce52197259505b0445735a86c3","unresolved":false,"context_lines":[{"line_number":133,"context_line":"                                parsed, new_access_key, new_secret_key)"},{"line_number":134,"context_line":"                            # Save the image immediately after update"},{"line_number":135,"context_line":"                            self.image_repo.save(image)"},{"line_number":136,"context_line":"                            break"},{"line_number":137,"context_line":"        return image"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"4dd0f7d7_39cf3779","line":136,"in_reply_to":"b0ee6068_6ef136e7","updated":"2025-10-23 21:12:32.000000000","message":"Ack","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"}],"glance/tests/unit/common/test_utils.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4df30dc156cc3b094b3f5b106d891aea7f072b29","unresolved":true,"context_lines":[{"line_number":1072,"context_line":"        self.image.locations \u003d [location]"},{"line_number":1073,"context_line":"        original_url \u003d location[\u0027url\u0027]"},{"line_number":1074,"context_line":"        store_utils.update_store_in_locations("},{"line_number":1075,"context_line":"            self.context, self.image, self.image_repo)"},{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":"        # URL should remain unchanged since credentials already match"},{"line_number":1078,"context_line":"        self.assertEqual(location[\u0027url\u0027], original_url)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7453b89d_adf56c47","line":1075,"updated":"2025-10-23 15:55:49.000000000","message":"Hmm, this should be taking the non-update path, returning False and not updating the image right? I think if you asserted this returns False, this would fail right?","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0c5ef5a76fa3f2b54293c666919523e02c58bceb","unresolved":false,"context_lines":[{"line_number":1072,"context_line":"        self.image.locations \u003d [location]"},{"line_number":1073,"context_line":"        original_url \u003d location[\u0027url\u0027]"},{"line_number":1074,"context_line":"        store_utils.update_store_in_locations("},{"line_number":1075,"context_line":"            self.context, self.image, self.image_repo)"},{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":"        # URL should remain unchanged since credentials already match"},{"line_number":1078,"context_line":"        self.assertEqual(location[\u0027url\u0027], original_url)"}],"source_content_type":"text/x-python","patch_set":7,"id":"8da84d5c_3c5cf95d","line":1075,"in_reply_to":"7453b89d_adf56c47","updated":"2025-10-23 20:13:31.000000000","message":"Ah, no, I misunderstood what this test was doing. This isn\u0027t attempting to cover the situation that is missing.","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4c483c7517057dce52197259505b0445735a86c3","unresolved":false,"context_lines":[{"line_number":1072,"context_line":"        self.image.locations \u003d [location]"},{"line_number":1073,"context_line":"        original_url \u003d location[\u0027url\u0027]"},{"line_number":1074,"context_line":"        store_utils.update_store_in_locations("},{"line_number":1075,"context_line":"            self.context, self.image, self.image_repo)"},{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":"        # URL should remain unchanged since credentials already match"},{"line_number":1078,"context_line":"        self.assertEqual(location[\u0027url\u0027], original_url)"}],"source_content_type":"text/x-python","patch_set":7,"id":"486e8baa_4df25f51","line":1075,"in_reply_to":"7453b89d_adf56c47","updated":"2025-10-23 21:12:32.000000000","message":"I have fixed the test to assert image save is not called.","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4df30dc156cc3b094b3f5b106d891aea7f072b29","unresolved":true,"context_lines":[{"line_number":1075,"context_line":"            self.context, self.image, self.image_repo)"},{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":"        # URL should remain unchanged since credentials already match"},{"line_number":1078,"context_line":"        self.assertEqual(location[\u0027url\u0027], original_url)"},{"line_number":1079,"context_line":""},{"line_number":1080,"context_line":"    @mock.patch(\u0027glance.common.store_utils._get_store_id_from_uri\u0027)"},{"line_number":1081,"context_line":"    @mock.patch(\u0027glance.common.store_utils.store_api\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"06a73b95_5b34e8dc","line":1078,"updated":"2025-10-23 15:55:49.000000000","message":"This is not a strong enough assertion that we\u0027re not doing the update, it\u0027s just asserting that it\u0027s as expected, but we might have falsely updated it to the same thing...","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0c5ef5a76fa3f2b54293c666919523e02c58bceb","unresolved":true,"context_lines":[{"line_number":1075,"context_line":"            self.context, self.image, self.image_repo)"},{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":"        # URL should remain unchanged since credentials already match"},{"line_number":1078,"context_line":"        self.assertEqual(location[\u0027url\u0027], original_url)"},{"line_number":1079,"context_line":""},{"line_number":1080,"context_line":"    @mock.patch(\u0027glance.common.store_utils._get_store_id_from_uri\u0027)"},{"line_number":1081,"context_line":"    @mock.patch(\u0027glance.common.store_utils.store_api\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"2643e560_d459f336","line":1078,"in_reply_to":"06a73b95_5b34e8dc","updated":"2025-10-23 20:13:31.000000000","message":"I was thinking we could assert the result to know if we did the update or not, but I see it\u0027s not returned from `update_store_in_locations()`.. so we should probably passthrough-mock `image.save()` or something.","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4c483c7517057dce52197259505b0445735a86c3","unresolved":false,"context_lines":[{"line_number":1075,"context_line":"            self.context, self.image, self.image_repo)"},{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":"        # URL should remain unchanged since credentials already match"},{"line_number":1078,"context_line":"        self.assertEqual(location[\u0027url\u0027], original_url)"},{"line_number":1079,"context_line":""},{"line_number":1080,"context_line":"    @mock.patch(\u0027glance.common.store_utils._get_store_id_from_uri\u0027)"},{"line_number":1081,"context_line":"    @mock.patch(\u0027glance.common.store_utils.store_api\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"8937fe1d_b6080c17","line":1078,"in_reply_to":"06a73b95_5b34e8dc","updated":"2025-10-23 21:12:32.000000000","message":"ditto","commit_id":"53999b900080041720bf2ead9f7efc858818c3e5"}],"releasenotes/notes/bug-2127798-ff4ecc0c566093da.yaml":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a7d708bc2b84512de8ad3c014c186ce98cba2077","unresolved":true,"context_lines":[{"line_number":4,"context_line":"    Bug 2127798_: Automatic S3 credential update when EC2 credentials are rotated"},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"    Glance now automatically updates S3 image location URLs when EC2 credentials"},{"line_number":7,"context_line":"    are rotated in Keystone. Previously, images with S3 locations would become"},{"line_number":8,"context_line":"    inaccessible after credential rotation, requiring manual intervention to update"},{"line_number":9,"context_line":"    location metadata. This fix ensures seamless credential rotation without"},{"line_number":10,"context_line":"    breaking image access."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"cb5eefc9_5bf06747","line":7,"updated":"2025-10-20 15:53:06.000000000","message":"...only for multi-store cases right? For the single-store case it seems like you only ever update the name.","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"206bdf6d4f1ef3ac3560bb4855c2682a571381af","unresolved":true,"context_lines":[{"line_number":4,"context_line":"    Bug 2127798_: Automatic S3 credential update when EC2 credentials are rotated"},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"    Glance now automatically updates S3 image location URLs when EC2 credentials"},{"line_number":7,"context_line":"    are rotated in Keystone. Previously, images with S3 locations would become"},{"line_number":8,"context_line":"    inaccessible after credential rotation, requiring manual intervention to update"},{"line_number":9,"context_line":"    location metadata. This fix ensures seamless credential rotation without"},{"line_number":10,"context_line":"    breaking image access."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"e2a798cf_eb54e3af","line":7,"in_reply_to":"32909917_ea69b800","updated":"2025-10-20 16:23:31.000000000","message":"Ah, I see, that\u0027s different from the first version and I was trying to look at the delta. I guess I\u0027m not sure why we need to do that in the separate place...","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3ef9c0532fb70b5bc04f30f424c9d17b9d5fa85d","unresolved":true,"context_lines":[{"line_number":4,"context_line":"    Bug 2127798_: Automatic S3 credential update when EC2 credentials are rotated"},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"    Glance now automatically updates S3 image location URLs when EC2 credentials"},{"line_number":7,"context_line":"    are rotated in Keystone. Previously, images with S3 locations would become"},{"line_number":8,"context_line":"    inaccessible after credential rotation, requiring manual intervention to update"},{"line_number":9,"context_line":"    location metadata. This fix ensures seamless credential rotation without"},{"line_number":10,"context_line":"    breaking image access."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"32909917_ea69b800","line":7,"in_reply_to":"cb5eefc9_5bf06747","updated":"2025-10-20 16:19:49.000000000","message":"I have covered credentials mismatch case for single store in location.py","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"c5defebd7a45b3bf8a1cfe5f6613c89a40496ed2","unresolved":true,"context_lines":[{"line_number":4,"context_line":"    Bug 2127798_: Automatic S3 credential update when EC2 credentials are rotated"},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"    Glance now automatically updates S3 image location URLs when EC2 credentials"},{"line_number":7,"context_line":"    are rotated in Keystone. Previously, images with S3 locations would become"},{"line_number":8,"context_line":"    inaccessible after credential rotation, requiring manual intervention to update"},{"line_number":9,"context_line":"    location metadata. This fix ensures seamless credential rotation without"},{"line_number":10,"context_line":"    breaking image access."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"fc5774fe_82e14281","line":7,"in_reply_to":"e2a798cf_eb54e3af","updated":"2025-10-20 17:58:27.000000000","message":"The reason is earlier i forgot that i was doing change inside  condition where it was inside condition of enabled_backends only where single store code was not reachable at all.","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7321535d7a54671382ea2543cc9e3295f5664940","unresolved":false,"context_lines":[{"line_number":4,"context_line":"    Bug 2127798_: Automatic S3 credential update when EC2 credentials are rotated"},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"    Glance now automatically updates S3 image location URLs when EC2 credentials"},{"line_number":7,"context_line":"    are rotated in Keystone. Previously, images with S3 locations would become"},{"line_number":8,"context_line":"    inaccessible after credential rotation, requiring manual intervention to update"},{"line_number":9,"context_line":"    location metadata. This fix ensures seamless credential rotation without"},{"line_number":10,"context_line":"    breaking image access."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"3a7e9711_c970ebd5","line":7,"in_reply_to":"fc5774fe_82e14281","updated":"2025-10-21 06:51:56.000000000","message":"Done","commit_id":"4dcc4dd6e9324fb6fe4dd627fb288ef75d532714"}]}