)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":32704,"name":"Alfredo Garcia","display_name":"Alfredo Garcia","email":"alfrgarc@redhat.com","username":"alfrgarc","status":"Senior Software Quality Engineer @ Red Hat"},"change_message_id":"0b9ec3c9af275b34958ba8078a2a4a8e133442e1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"dafbccc0_af427ef8","updated":"2026-03-18 12:21:56.000000000","message":"A well written and well commented patch. LGTM.","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"125881c6c2d99cc56f45b74cad6404d078ce089f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"27c53125_ae75468c","updated":"2026-03-23 16:33:07.000000000","message":"Jason, I assume this is more review content from claude (or some other agent) which is again lacking a lot of context and understanding of the bigger picture here. I\u0027d ask you to please make sure the comments you\u0027re making are something you yourself think need to be made, especially in cases where the comment itself seems to be lacking enough information to even explain the suggestion (such as the container/container_format comment).","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"},{"author":{"_account_id":34373,"name":"Jason Paroly","email":"jparoly@redhat.com","username":"jparoly"},"change_message_id":"e9a05a6ba4651b53222cbb2be17bf9627b129666","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5de54455_56ca9aa9","in_reply_to":"27c53125_ae75468c","updated":"2026-03-23 17:33:37.000000000","message":"I did this review on my own.","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"}],"tempest/api/image/v2/test_images_formats.py":[{"author":{"_account_id":34373,"name":"Jason Paroly","email":"jparoly@redhat.com","username":"jparoly"},"change_message_id":"e8aa574e3592899afa6b08c2628df95affdeb006","unresolved":true,"context_lines":[{"line_number":75,"context_line":"            name\u003dimage_def[\u0027name\u0027])"},{"line_number":76,"context_line":"        image \u003d self.client.create_image("},{"line_number":77,"context_line":"            name\u003dimage_name,"},{"line_number":78,"context_line":"            container_format\u003dimage_def.get(\u0027container\u0027, \u0027bare\u0027),"},{"line_number":79,"context_line":"            disk_format\u003doverride_format or image_def[\u0027format\u0027])"},{"line_number":80,"context_line":"        self.images.append(image)"},{"line_number":81,"context_line":"        image_fn \u003d os.path.join(self._image_base, image_def[\u0027filename\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"12a93f63_971be13d","line":78,"range":{"start_line":78,"start_character":12,"end_line":78,"end_character":28},"updated":"2026-03-23 15:47:34.000000000","message":"nit: for consistency consider making \"container_format\" and \"container\" variables the same name.  They are getting the same value the same way.","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"125881c6c2d99cc56f45b74cad6404d078ce089f","unresolved":true,"context_lines":[{"line_number":75,"context_line":"            name\u003dimage_def[\u0027name\u0027])"},{"line_number":76,"context_line":"        image \u003d self.client.create_image("},{"line_number":77,"context_line":"            name\u003dimage_name,"},{"line_number":78,"context_line":"            container_format\u003dimage_def.get(\u0027container\u0027, \u0027bare\u0027),"},{"line_number":79,"context_line":"            disk_format\u003doverride_format or image_def[\u0027format\u0027])"},{"line_number":80,"context_line":"        self.images.append(image)"},{"line_number":81,"context_line":"        image_fn \u003d os.path.join(self._image_base, image_def[\u0027filename\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"1b79dfea_c162b955","line":78,"range":{"start_line":78,"start_character":12,"end_line":78,"end_character":28},"in_reply_to":"12a93f63_971be13d","updated":"2026-03-23 16:33:07.000000000","message":"Sorry, what? There\u0027s no \"container\" variable here. The parameter \"container_format\" matches the actual image property of the same name, and the \"container\" string is defined in a separate project\u0027s manifest of image information.\n\nIs this more claude-provided review content?","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"},{"author":{"_account_id":34373,"name":"Jason Paroly","email":"jparoly@redhat.com","username":"jparoly"},"change_message_id":"e9a05a6ba4651b53222cbb2be17bf9627b129666","unresolved":true,"context_lines":[{"line_number":75,"context_line":"            name\u003dimage_def[\u0027name\u0027])"},{"line_number":76,"context_line":"        image \u003d self.client.create_image("},{"line_number":77,"context_line":"            name\u003dimage_name,"},{"line_number":78,"context_line":"            container_format\u003dimage_def.get(\u0027container\u0027, \u0027bare\u0027),"},{"line_number":79,"context_line":"            disk_format\u003doverride_format or image_def[\u0027format\u0027])"},{"line_number":80,"context_line":"        self.images.append(image)"},{"line_number":81,"context_line":"        image_fn \u003d os.path.join(self._image_base, image_def[\u0027filename\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f9284ac_c4447cf0","line":78,"range":{"start_line":78,"start_character":12,"end_line":78,"end_character":28},"in_reply_to":"1b79dfea_c162b955","updated":"2026-03-23 17:33:37.000000000","message":"In the test case that is part of this PR you are using \"container\" as a variable name.  Sorry if I assumed the person making this change would have that context. line 97 andd line 78 are making the same call.","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"09f978d8848f7cb5094a307c061068f4167e354c","unresolved":true,"context_lines":[{"line_number":75,"context_line":"            name\u003dimage_def[\u0027name\u0027])"},{"line_number":76,"context_line":"        image \u003d self.client.create_image("},{"line_number":77,"context_line":"            name\u003dimage_name,"},{"line_number":78,"context_line":"            container_format\u003dimage_def.get(\u0027container\u0027, \u0027bare\u0027),"},{"line_number":79,"context_line":"            disk_format\u003doverride_format or image_def[\u0027format\u0027])"},{"line_number":80,"context_line":"        self.images.append(image)"},{"line_number":81,"context_line":"        image_fn \u003d os.path.join(self._image_base, image_def[\u0027filename\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"98f65f56_27ace734","line":78,"range":{"start_line":78,"start_character":12,"end_line":78,"end_character":28},"in_reply_to":"7f9284ac_c4447cf0","updated":"2026-03-23 17:47:25.000000000","message":"Here we are not setting a variable named \"container_format\". We are asking `create_image()` to set the `container_format` image property to the value of the \"container\" as specified in the image_def.\n\n\u003e Sorry if I assumed the person making this change would have that context.\n\nIf you comment on a line and suggest a change, it\u0027s sort of implied that you\u0027re expecting a change on that line.\n\n\u003e line 97 andd line 78 are making the same call.\n\nThey\u0027re both retrieving the same item from the image def, but line 78 is passing it to a _call_ that has a parameter of `container_format`. On line 97, we\u0027re examining the `container` item from the image_def spec. It\u0027s called container there. It specifies the container, and that schema is defined by that other project. Using a variable of \"container\" to refer to the \"container\" seems appropriate to me, even if we use the value of \"container\" to inform the \"container_format\" image property on line 78.","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"},{"author":{"_account_id":34373,"name":"Jason Paroly","email":"jparoly@redhat.com","username":"jparoly"},"change_message_id":"a056ad177fe82b7219f76040f23517208b8e200c","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            name\u003dimage_def[\u0027name\u0027])"},{"line_number":76,"context_line":"        image \u003d self.client.create_image("},{"line_number":77,"context_line":"            name\u003dimage_name,"},{"line_number":78,"context_line":"            container_format\u003dimage_def.get(\u0027container\u0027, \u0027bare\u0027),"},{"line_number":79,"context_line":"            disk_format\u003doverride_format or image_def[\u0027format\u0027])"},{"line_number":80,"context_line":"        self.images.append(image)"},{"line_number":81,"context_line":"        image_fn \u003d os.path.join(self._image_base, image_def[\u0027filename\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"d5ec735d_823cf3c6","line":78,"range":{"start_line":78,"start_character":12,"end_line":78,"end_character":28},"in_reply_to":"98f65f56_27ace734","updated":"2026-03-23 17:52:22.000000000","message":"Acknowledged","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"},{"author":{"_account_id":34373,"name":"Jason Paroly","email":"jparoly@redhat.com","username":"jparoly"},"change_message_id":"e8aa574e3592899afa6b08c2628df95affdeb006","unresolved":true,"context_lines":[{"line_number":94,"context_line":"                self._test_image(self.imgdef)"},{"line_number":95,"context_line":"            except lib_exc.BadRequest:"},{"line_number":96,"context_line":"                format \u003d self.imgdef[\u0027format\u0027]"},{"line_number":97,"context_line":"                container \u003d self.imgdef.get(\u0027container\u0027, \u0027bare\u0027)"},{"line_number":98,"context_line":"                if format \u003d\u003d \u0027gpt\u0027 and format not in CONF.image.disk_formats:"},{"line_number":99,"context_line":"                    # If we don\u0027t have gpt defined, we don\u0027t expect this to"},{"line_number":100,"context_line":"                    # work because glance has not been updated for GPT"}],"source_content_type":"text/x-python","patch_set":3,"id":"81dd76d1_7cb7ff43","line":97,"updated":"2026-03-23 15:47:34.000000000","message":"Consider moving the checks before testing the image because it seems you already know it will fail.  Could save some time in the test to check and skip the test prior to testing the image.","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"},{"author":{"_account_id":34373,"name":"Jason Paroly","email":"jparoly@redhat.com","username":"jparoly"},"change_message_id":"a056ad177fe82b7219f76040f23517208b8e200c","unresolved":false,"context_lines":[{"line_number":94,"context_line":"                self._test_image(self.imgdef)"},{"line_number":95,"context_line":"            except lib_exc.BadRequest:"},{"line_number":96,"context_line":"                format \u003d self.imgdef[\u0027format\u0027]"},{"line_number":97,"context_line":"                container \u003d self.imgdef.get(\u0027container\u0027, \u0027bare\u0027)"},{"line_number":98,"context_line":"                if format \u003d\u003d \u0027gpt\u0027 and format not in CONF.image.disk_formats:"},{"line_number":99,"context_line":"                    # If we don\u0027t have gpt defined, we don\u0027t expect this to"},{"line_number":100,"context_line":"                    # work because glance has not been updated for GPT"}],"source_content_type":"text/x-python","patch_set":3,"id":"8bbcda72_af3c1b63","line":97,"in_reply_to":"38626446_ea1cd827","updated":"2026-03-23 17:52:22.000000000","message":"Acknowledged","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"},{"author":{"_account_id":34373,"name":"Jason Paroly","email":"jparoly@redhat.com","username":"jparoly"},"change_message_id":"e9a05a6ba4651b53222cbb2be17bf9627b129666","unresolved":true,"context_lines":[{"line_number":94,"context_line":"                self._test_image(self.imgdef)"},{"line_number":95,"context_line":"            except lib_exc.BadRequest:"},{"line_number":96,"context_line":"                format \u003d self.imgdef[\u0027format\u0027]"},{"line_number":97,"context_line":"                container \u003d self.imgdef.get(\u0027container\u0027, \u0027bare\u0027)"},{"line_number":98,"context_line":"                if format \u003d\u003d \u0027gpt\u0027 and format not in CONF.image.disk_formats:"},{"line_number":99,"context_line":"                    # If we don\u0027t have gpt defined, we don\u0027t expect this to"},{"line_number":100,"context_line":"                    # work because glance has not been updated for GPT"}],"source_content_type":"text/x-python","patch_set":3,"id":"be5e2827_ab0ebfa9","line":97,"in_reply_to":"68510dce_fec61e97","updated":"2026-03-23 17:33:37.000000000","message":"Isn\u0027t that what the try is doing.  If \"self.imgdef.get(\u0027container\u0027, \u0027bare\u0027)\" returns \"luks\" and \"container not in CONF.image.container_formats\" then we skip the test.  It will only go into the except if the try exceptions out due to lib_exc.BadRequest, which I assume means the self._test_image() throws and exception.  It seems we will know it will fail so why let it exception out?","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"125881c6c2d99cc56f45b74cad6404d078ce089f","unresolved":true,"context_lines":[{"line_number":94,"context_line":"                self._test_image(self.imgdef)"},{"line_number":95,"context_line":"            except lib_exc.BadRequest:"},{"line_number":96,"context_line":"                format \u003d self.imgdef[\u0027format\u0027]"},{"line_number":97,"context_line":"                container \u003d self.imgdef.get(\u0027container\u0027, \u0027bare\u0027)"},{"line_number":98,"context_line":"                if format \u003d\u003d \u0027gpt\u0027 and format not in CONF.image.disk_formats:"},{"line_number":99,"context_line":"                    # If we don\u0027t have gpt defined, we don\u0027t expect this to"},{"line_number":100,"context_line":"                    # work because glance has not been updated for GPT"}],"source_content_type":"text/x-python","patch_set":3,"id":"68510dce_fec61e97","line":97,"in_reply_to":"81dd76d1_7cb7ff43","updated":"2026-03-23 16:33:07.000000000","message":"No, we don\u0027t already know it will fail. There\u0027s an ambiguity here in that we don\u0027t know the patch level of the glance we\u0027re testing against. Just like the gpt case below (and the comments for both gpt and luks) we are accounting for the situation(s) where we don\u0027t actually know if this will fail, but are tolerating what would have been correct behavior for an earlier version of glance.","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"09f978d8848f7cb5094a307c061068f4167e354c","unresolved":true,"context_lines":[{"line_number":94,"context_line":"                self._test_image(self.imgdef)"},{"line_number":95,"context_line":"            except lib_exc.BadRequest:"},{"line_number":96,"context_line":"                format \u003d self.imgdef[\u0027format\u0027]"},{"line_number":97,"context_line":"                container \u003d self.imgdef.get(\u0027container\u0027, \u0027bare\u0027)"},{"line_number":98,"context_line":"                if format \u003d\u003d \u0027gpt\u0027 and format not in CONF.image.disk_formats:"},{"line_number":99,"context_line":"                    # If we don\u0027t have gpt defined, we don\u0027t expect this to"},{"line_number":100,"context_line":"                    # work because glance has not been updated for GPT"}],"source_content_type":"text/x-python","patch_set":3,"id":"38626446_ea1cd827","line":97,"in_reply_to":"be5e2827_ab0ebfa9","updated":"2026-03-23 17:47:25.000000000","message":"We\u0027re testing against a remote glance here. A glance that we do not control, and which may or may not have the new luks behavior. It\u0027s not guaranteed this will fail. It will fail if we are running against an older glance, and if it does fail in a very specific way, then it means that glance is too old for us to perform this check and thus we skip. If it failed that way for something other than luks (or GPT, see the exact same scenario just above) then it would be a problem and we should not skip.","commit_id":"da73e61bbffa42509956c7813ac9805042bdfd49"}]}
