)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"45939021f1db8d4ba7edd20ea831525f105399c7","unresolved":true,"context_lines":[{"line_number":10,"context_line":"that is based on an image back to glance. This bug is triggered if"},{"line_number":11,"context_line":"glance multistore is enabled (devstack in this example)."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"When enabling multistore, the following properties will be stored in Cinder:"},{"line_number":14,"context_line":"* os_glance_failed_import\u003d\u0027\u0027"},{"line_number":15,"context_line":"* os_glance_importing_to_stores\u003d\u0027\u0027"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"e9520253_641b3ad9","line":13,"range":{"start_line":13,"start_character":65,"end_line":13,"end_character":76},"updated":"2021-10-06 20:08:10.000000000","message":":-1: please consider adjusting the text lines to 79 characters, in this way it\u0027s easier to read.","commit_id":"eceaa95aa2f2b46d16df40f2af97d4b6d434842a"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"9505ab8453c1e28c5fd4d270d7fba03622f64072","unresolved":false,"context_lines":[{"line_number":10,"context_line":"that is based on an image back to glance. This bug is triggered if"},{"line_number":11,"context_line":"glance multistore is enabled (devstack in this example)."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"When enabling multistore, the following properties will be stored in Cinder:"},{"line_number":14,"context_line":"* os_glance_failed_import\u003d\u0027\u0027"},{"line_number":15,"context_line":"* os_glance_importing_to_stores\u003d\u0027\u0027"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"4f399082_74a1409d","line":13,"range":{"start_line":13,"start_character":65,"end_line":13,"end_character":76},"in_reply_to":"e9520253_641b3ad9","updated":"2021-10-07 12:19:53.000000000","message":"This line has 77 characters.","commit_id":"eceaa95aa2f2b46d16df40f2af97d4b6d434842a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6220a1c9bd2b5e51910d4bbf97192da92d3cf002","unresolved":true,"context_lines":[{"line_number":11,"context_line":"glance multistore is enabled (devstack in this example)."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"When enabling multistore, the following properties will be stored in Cinder:"},{"line_number":14,"context_line":"* os_glance_failed_import\u003d\u0027\u0027"},{"line_number":15,"context_line":"* os_glance_importing_to_stores\u003d\u0027\u0027"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Those properties will cause problems when later, Cinder tries to perform"},{"line_number":18,"context_line":"some actions with Glance. Error msg:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"13afd5a5_5d0b847d","line":15,"range":{"start_line":14,"start_character":0,"end_line":15,"end_character":34},"updated":"2021-10-07 12:00:08.000000000","message":"These are tracking properties for Glance ... I wonder whether we should be excluding them on the volume-create instead of the volume-upload-to-image?  On the other hand, we\u0027ve already got them in the database, and I think we\u0027ll need to backport this fix to victoria (where I think glance multistore is introduced), so we don\u0027t want to do anything that would require a database migration.","commit_id":"eceaa95aa2f2b46d16df40f2af97d4b6d434842a"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"9505ab8453c1e28c5fd4d270d7fba03622f64072","unresolved":true,"context_lines":[{"line_number":11,"context_line":"glance multistore is enabled (devstack in this example)."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"When enabling multistore, the following properties will be stored in Cinder:"},{"line_number":14,"context_line":"* os_glance_failed_import\u003d\u0027\u0027"},{"line_number":15,"context_line":"* os_glance_importing_to_stores\u003d\u0027\u0027"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Those properties will cause problems when later, Cinder tries to perform"},{"line_number":18,"context_line":"some actions with Glance. Error msg:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa7c0d84_d2973e66","line":15,"range":{"start_line":14,"start_character":0,"end_line":15,"end_character":34},"in_reply_to":"13afd5a5_5d0b847d","updated":"2021-10-07 12:19:53.000000000","message":"Exactly. That is why I did in both places.","commit_id":"eceaa95aa2f2b46d16df40f2af97d4b6d434842a"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"102e085b0d0865c383af4ddd35f220880f9a713a","unresolved":false,"context_lines":[{"line_number":11,"context_line":"glance multistore is enabled (devstack in this example)."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"When enabling multistore, the following properties will be stored in Cinder:"},{"line_number":14,"context_line":"* os_glance_failed_import\u003d\u0027\u0027"},{"line_number":15,"context_line":"* os_glance_importing_to_stores\u003d\u0027\u0027"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Those properties will cause problems when later, Cinder tries to perform"},{"line_number":18,"context_line":"some actions with Glance. Error msg:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7286e939_6c656546","line":15,"range":{"start_line":14,"start_character":0,"end_line":15,"end_character":34},"in_reply_to":"fa7c0d84_d2973e66","updated":"2022-06-29 11:51:14.000000000","message":"Done","commit_id":"eceaa95aa2f2b46d16df40f2af97d4b6d434842a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"38216a023f1a4acd544376673935549f80b723db","unresolved":true,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"https://github.com/openstack/nova/commit/dda179d3f901e4f23091f3095f1af58bc26e222e"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Therefore, this patch is intended to apply a similar solution in Cinder."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Change-Id: I79d70543856c01a45e2d8c083ab8df6b9c047ebc"},{"line_number":33,"context_line":"Closes-Bug: #1945500"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"e3c6f1ac_199b9ed6","line":30,"updated":"2022-11-10 14:08:18.000000000","message":"I think the Nova reference is misleading here, because I\u0027m pretty sure the Nova solution does the filtering on upload (i.e., the offending properties are stored in the instance\u0027s image metadata, but aren\u0027t copied when a user requests a snapshot of that instance).  Your solution in this patch excludes the properties on download (i.e., cinder will never store them in the first place).","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"7438129a446423215b97adb1ff52edef4d4d4239","unresolved":true,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"https://github.com/openstack/nova/commit/dda179d3f901e4f23091f3095f1af58bc26e222e"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Therefore, this patch is intended to apply a similar solution in Cinder."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Change-Id: I79d70543856c01a45e2d8c083ab8df6b9c047ebc"},{"line_number":33,"context_line":"Closes-Bug: #1945500"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"4fcbb194_b6a2c0de","line":30,"in_reply_to":"0f9e9cb6_fe1c41f3","updated":"2022-11-11 13:58:32.000000000","message":"Just to clarify what my understanding was of Brian\u0027s suggestion, and therefore better contextualize my response, I am in favor of not deleting the metadata when creating volumes, just when performing operations that would break it (such as upload-to-glance). Given that it has already been coded the filtering of metadata when volumes are created, we can move forward as-is, as I don\u0027t really mind it and the metadata is useless to Cinder, or we could decide to remove it without much effort. Either way, not much effort is needed, but we need to decide what is best.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"17694ac4b4d8431228c5ab219656651d38123391","unresolved":true,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"https://github.com/openstack/nova/commit/dda179d3f901e4f23091f3095f1af58bc26e222e"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Therefore, this patch is intended to apply a similar solution in Cinder."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Change-Id: I79d70543856c01a45e2d8c083ab8df6b9c047ebc"},{"line_number":33,"context_line":"Closes-Bug: #1945500"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"0f9e9cb6_fe1c41f3","line":30,"in_reply_to":"4c66428d_11c3d692","updated":"2022-11-11 13:48:24.000000000","message":"It is not misleading. It is a similar approach (similar!\u003dsame!\u003dequal). I used the idea of filtering they applied there. I am not doing the same as they do in Nova. \n\nWe can remove this sentence, if you think it is important to remove it though.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e3a23c7eb36cf8587f83ecd767bcac517541c4cb","unresolved":true,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"https://github.com/openstack/nova/commit/dda179d3f901e4f23091f3095f1af58bc26e222e"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Therefore, this patch is intended to apply a similar solution in Cinder."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Change-Id: I79d70543856c01a45e2d8c083ab8df6b9c047ebc"},{"line_number":33,"context_line":"Closes-Bug: #1945500"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"775d277c_7592969a","line":30,"in_reply_to":"4fcbb194_b6a2c0de","updated":"2022-11-15 14:41:42.000000000","message":"We can discuss this at the weekly cinder meeting tomorrow, but my opinion is that saving the image properties is important because some of them are actionable, and they can be changed on the image record in Glance.  For example, I create a volume V1 from image 1 at time t that has the image property cinder_img_volume_type\u003dFred.  Volume V1 is created with volume_type Fred.  At time t+1, the image owner removes the \u0027cinder_img_volume_type\u0027 property from the image.  At time t+2, I create a volume V2 from image 1, but this one has volume_type DEFAULT.  I call support and ask WTF.  Without a record of what the image properties were at the time V1 was created, this will be difficult to troubleshoot.\n\nSo I think we should do what nova does here and filter out properties on upload, even the os_glance ones.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"25fbbe4bfbe0554b5e5237e8a0e00f3841fdd4ea","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"https://github.com/openstack/nova/commit/dda179d3f901e4f23091f3095f1af58bc26e222e"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Therefore, this patch is intended to apply a similar solution in Cinder."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Change-Id: I79d70543856c01a45e2d8c083ab8df6b9c047ebc"},{"line_number":33,"context_line":"Closes-Bug: #1945500"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"bdb508b2_443b85fb","line":30,"in_reply_to":"5552f18d_24ca936a","updated":"2022-11-24 14:52:01.000000000","message":"Done","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"0b137a1e0c0d0424eb6aa0da864c704d896d9107","unresolved":true,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"https://github.com/openstack/nova/commit/dda179d3f901e4f23091f3095f1af58bc26e222e"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Therefore, this patch is intended to apply a similar solution in Cinder."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Change-Id: I79d70543856c01a45e2d8c083ab8df6b9c047ebc"},{"line_number":33,"context_line":"Closes-Bug: #1945500"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"5552f18d_24ca936a","line":30,"in_reply_to":"775d277c_7592969a","updated":"2022-11-15 17:33:27.000000000","message":"+1 to only removing on upload-back-to-glance","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2993b8aafaeef5c1443701995168de839bb3f8bb","unresolved":true,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"https://github.com/openstack/nova/commit/dda179d3f901e4f23091f3095f1af58bc26e222e"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Therefore, this patch is intended to apply a similar solution in Cinder."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Change-Id: I79d70543856c01a45e2d8c083ab8df6b9c047ebc"},{"line_number":33,"context_line":"Closes-Bug: #1945500"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"4c66428d_11c3d692","line":30,"in_reply_to":"e3c6f1ac_199b9ed6","updated":"2022-11-10 20:01:27.000000000","message":"I personally agree with your suggestion, but I am not the author and I am just trying to move this patch forward, if everyone accepts the approach. So I\u0027m not making that drastic change myself before I hear from the original author, Rafael, and have general agreement on whether we should or should not prevent the metadata from being saved on volume creation.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"e354a1a9643532442928aaf94cc22450a631e35a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"c0ee1ccd_8203ad94","updated":"2022-02-26 00:00:23.000000000","message":"Hello guys, \nDo we have an update on this patch?","commit_id":"cb073626bef2267f883afd5ec1b34cd95b1d9a17"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"3ec01300da456de6b610902d844032a325bc7f09","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ed940634_c8626f12","updated":"2022-01-26 15:32:31.000000000","message":"Hello guys, \nDo we have an update on this patch?","commit_id":"cb073626bef2267f883afd5ec1b34cd95b1d9a17"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"bd3ec309e20eeb3fcfa0be902734a0e671428ac1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"849f53b1_99a4fb26","updated":"2022-02-14 22:13:08.000000000","message":"Hello guys, \nDo we have an update on this patch?\n\nThis is an important patch to be added into Cinder.","commit_id":"cb073626bef2267f883afd5ec1b34cd95b1d9a17"},{"author":{"_account_id":28801,"name":"Cisco Cinder CI","email":"cisco-cinder-ci@cisco.com","username":"cisco-cinder-ci"},"change_message_id":"6dde87e67191e9b0983f81398e0cfb13111806f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"f50c1026_c9a30556","updated":"2021-12-17 08:17:17.000000000","message":"cisco-cinder-ci","commit_id":"cb073626bef2267f883afd5ec1b34cd95b1d9a17"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"a88b1df4e0d37ee954f6a4f7a557fe324fa240fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"31f2187e_abc2e490","updated":"2022-03-04 00:06:25.000000000","message":"recheck","commit_id":"cb073626bef2267f883afd5ec1b34cd95b1d9a17"},{"author":{"_account_id":34609,"name":"Maxim Monin","email":"maximmonin@gmail.com","username":"maximmonin"},"change_message_id":"9dfa40605d486d7d4ff1192ebf6deb49dd3bd003","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"b5307019_e769491c","updated":"2022-03-04 05:12:01.000000000","message":"right now w/o patch issue can be fixed with calling api method:\ncinder/{project_id}/volumes/{volume_id}/action\nwith\n{\n    \"os-unset_image_metadata\": {\n        \"key\": \"os_glance_failed_import\"\n    }\n}\nand with\n{\n    \"os-unset_image_metadata\": {\n        \"key\": \"os_glance_importing_to_stores\"\n    }\n}\nand ignoring 404 Item not found","commit_id":"cb073626bef2267f883afd5ec1b34cd95b1d9a17"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"c3fa69139698a555550be22c85286adea7cfd376","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"a61b7563_6aa6b843","in_reply_to":"b5307019_e769491c","updated":"2022-03-04 10:42:04.000000000","message":"Are you suggesting to change the request then?\n\nI would rather just filter them out as Nova did, because this can happen with other components that we integrate with, and they might not have such an API as glance to ignore some tags. However, if you guys strongly think that this is the best approach I could implement it then.\n\nBTW: it is a rather unexpected API call to me, to ask the API to unset some variables in the response.","commit_id":"cb073626bef2267f883afd5ec1b34cd95b1d9a17"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"bcd6c19cddd96da898442eeabc3c1f3f68e58615","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"9bc8ce2c_5c3e8d71","updated":"2022-06-22 17:42:57.000000000","message":"Thanks for the review!","commit_id":"c08104af0b4fb0cc7ca710e56d4e77c0fa643b19"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a979a1d81a5619b7700ee80185d92a2d65c1c1b7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"80631ed3_b201e067","updated":"2022-06-22 17:12:22.000000000","message":"This is the right approach, i have noted few nits inside but looks good to me.","commit_id":"c08104af0b4fb0cc7ca710e56d4e77c0fa643b19"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"a32883f798c7eadf12c63d23ff9e11d8436a29da","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"fb933506_94e81058","updated":"2022-06-29 01:09:51.000000000","message":"Hey, thanks for working on this. \n\nI am voting +1 to get people\u0027s attention, as this looks like a solution we want to merge soon. However, there are still a few outstanding issues that I would like to see resolved before we move forward.\n\nThis patch adds a configuration option among other things. Looks like a candidate for a release note. However, I\u0027d like to see what Brian thinks of it before asking for it. \nhttps://docs.openstack.org/cinder/latest/contributor/releasenotes.html","commit_id":"e7024dd09336319c3b413b42e3ba13cfcd28dc2e"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"102e085b0d0865c383af4ddd35f220880f9a713a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"fb36fb23_d15921a4","in_reply_to":"fb933506_94e81058","updated":"2022-06-29 11:51:14.000000000","message":"I added a release notes. Can you check it? And, about the outstanding issues, what would them be? The open reviews I closed as the reviewers did not reply back; I was just waiting for their feedback, but they never replied back.","commit_id":"e7024dd09336319c3b413b42e3ba13cfcd28dc2e"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"00a05685edd259f3e80ff127b51a154578e9bea9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"637d0b0a_02a2eb62","updated":"2022-10-12 11:46:14.000000000","message":"Thanks for the review Andrey! I have amended the code as suggested by you.","commit_id":"e47ee3f8d1f5c5b1cf3f68af7241baf1854e3853"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"9809c907895b0dd388754e082a8865208fca73ea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"e5362bcc_e9ccf3e3","updated":"2022-10-02 04:29:02.000000000","message":"looks good","commit_id":"e47ee3f8d1f5c5b1cf3f68af7241baf1854e3853"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"199731856753c01e9a3e18c848997a4172000227","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"adc742fd_09c212bb","updated":"2022-06-30 16:53:39.000000000","message":"recheck","commit_id":"e47ee3f8d1f5c5b1cf3f68af7241baf1854e3853"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"307aac69f919ec311b0fc7924f0c6ccc4b89e403","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"4a0510e2_84a213b5","updated":"2022-11-02 14:43:59.000000000","message":"Few comments inline, mostly questions and suggestions.","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"38216a023f1a4acd544376673935549f80b723db","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"346a7481_8b0db049","updated":"2022-11-10 14:08:18.000000000","message":"A few more comments inline.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2729cc1dc3a8406951a11c41c481c055fae38707","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"001e47fa_b3f72b64","updated":"2022-11-10 13:02:55.000000000","message":"One other issue that occurs to me is that you\u0027ve got test coverage of the filter function itself, but you don\u0027t have any coverage to make sure it\u0027s being called in the code paths where it should be.  I think you can find some current tests that can be modified to check for this.  It may be a matter of modifying the test volume-image-metadata so that something has to be filtered and then checking to make sure it\u0027s not there anymore, or it could be just mocking the filter function and asserting that the mock has been called.  The tests you have here show that the filter function works correctly, so the other tests just need to make sure it\u0027s actually being used where it should be.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"347fad509963aa776c906000383ccd696bdece39","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"8598caeb_5add06f2","updated":"2022-11-08 16:18:09.000000000","message":"Thanks Rajat for the review, please take a look at the latest patchset and the responses to your comments. ","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"64f14dd85fffb49f7f46d726e272a6585420a730","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"2531b08c_d1287b9b","updated":"2022-11-08 16:36:46.000000000","message":"Thanks Rodrigo!","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2993b8aafaeef5c1443701995168de839bb3f8bb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"f9326b62_9823847a","updated":"2022-11-10 20:01:27.000000000","message":"Thanks for the reviews Rajat and Brian! I addressed the release notes and unit tests, and added comments for the remaining ones, which I believe need further discussion/clarification. Also, I would like to hear back from Rafael on this.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ec62702b0e248cafc738c3a5fd93d57c83a8a65b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"8b46626f_483b28e4","updated":"2022-11-09 23:44:51.000000000","message":"This is looking good.  I have a few suggestions for the unit tests, and a question in the filter function.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"eb57210640f582639c299f97ae05cc4ecfcf2660","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"5158f9d1_1dc9fb26","updated":"2022-11-10 08:15:19.000000000","message":"looking better. few more comments inline.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"17694ac4b4d8431228c5ab219656651d38123391","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"a6b4dc9a_e1e5cca6","updated":"2022-11-11 13:48:24.000000000","message":"Thank you guys for reviewing the patch so promptly. I tried to help Rodrigo whenever I could here.\n\nAlso, than you Rodrigo for picking this one up. If you need, I can amend the patch with the follow ups. I only left it sopped due to the slowness of interaction whenever I was replying and amending the code due to reviews.","commit_id":"2c296ba642c0c20bfeefa82f6235ff5f326f41c3"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e3a23c7eb36cf8587f83ecd767bcac517541c4cb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"d2645373_089af5fe","updated":"2022-11-15 14:41:42.000000000","message":"Comments inline.","commit_id":"f32d2bf66a081cf8b34042c944f60cc1fe3faf5c"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"e3bf34ea94384dac34f1ef04cf2beef88d9d7fe6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"8def68b1_70fc9e6b","updated":"2022-11-15 17:37:27.000000000","message":"Rodrigo, so we coordinate. I was applying the changes Brian requested. However, I just saw your comment here. Should I assume that you took over, and all of the next changes/modifications will be done by you?","commit_id":"f32d2bf66a081cf8b34042c944f60cc1fe3faf5c"},{"author":{"_account_id":28801,"name":"Cisco Cinder CI","email":"cisco-cinder-ci@cisco.com","username":"cisco-cinder-ci"},"change_message_id":"965107233fbd567ad5c7d54b6ee1c5c01ce36ce8","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":20,"id":"ea37b0dd_4f56a745","updated":"2022-11-14 05:51:00.000000000","message":"cisco-cinder-ci","commit_id":"f32d2bf66a081cf8b34042c944f60cc1fe3faf5c"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"619a318907fd6c4575b3c55e90446f2b6e3b4f1f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"5a44ba4c_a613491d","in_reply_to":"8def68b1_70fc9e6b","updated":"2022-11-15 21:28:04.000000000","message":"Hey Rafael. I haven\u0027t put any efforts into the changes requested since my latest patchset. I was going to wait for tomorrow\u0027s weekly meeting for a decision, but if we are all in agreement with the suggested changes then I suppose we don\u0027t have much to discuss, so please feel free to make the changes and push new patchsets, I have not took over. Thanks for the updates!","commit_id":"f32d2bf66a081cf8b34042c944f60cc1fe3faf5c"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"5522f3fe554b0adb619011227c161e04e0b1cb98","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"d442952a_e9268db1","in_reply_to":"bab1ebaa_7308afea","updated":"2022-12-08 14:26:44.000000000","message":"Done","commit_id":"f32d2bf66a081cf8b34042c944f60cc1fe3faf5c"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"b3e23fb06c50cc27395e86612616dd6aadcc26cc","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":20,"id":"bab1ebaa_7308afea","in_reply_to":"ea37b0dd_4f56a745","updated":"2022-11-28 13:19:49.000000000","message":"I did not get this one","commit_id":"f32d2bf66a081cf8b34042c944f60cc1fe3faf5c"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"36bba509c64a6a188c9292b3a0ca80083b36e8f2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"91972406_c6f3436f","updated":"2022-11-30 10:41:36.000000000","message":"Hello Rodrigo, Brian, and others that have helped reviewing this patch. I have addressed your last suggestions here. Do we need to change something else before proceeding to merge?","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"36dfcf8335243116da9febc857a7e6c64ad1115b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"a992d0c9_8a116399","updated":"2022-11-28 12:58:18.000000000","message":"Hi Rafael, just to clarify, patchset 23 is just a rebase. You\u0027ve marked the previous comments as \"Done\", but there the changes are not in the latest patchset (23). Is there a patchset pending to be submitted?","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"abe15d667661b5e29b42d4aa2d4778884b6dba28","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"a936d699_fd5e7069","updated":"2022-12-07 14:50:40.000000000","message":"Hi Rafael, we discussed the patch in today\u0027s meeting. See meetings notes:\n\nhttps://meetings.opendev.org/meetings/cinder/2022/cinder.2022-12-07-14.00.log.html#l-109\n\nAlso I got some comments inline in the release notes.","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2edc4b7d56f9450afffcbeb4cd9fe2b36e018960","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"36720015_5b782a42","updated":"2022-12-07 14:38:28.000000000","message":"Just a quick review, I will look more closely later.  Two primary issues at this point:\n\n1. I think you should have at least 2 items in the \"permanent\" reserved namespaces (see comment inline)\n\n2. The filter function doesn\u0027t seem to be used anywhere in the upload-volume-to-image workflow, where we really need it to avoid the bug you\u0027re fixing\n\n3. The unit tests are a bit hard to read due to the combination of hard-coded and configurable namespaces in the code (but that may be because i looked at them first, and didn\u0027t realize that there were 2 lists).\n\n","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"5522f3fe554b0adb619011227c161e04e0b1cb98","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"e9131442_abe14e1b","updated":"2022-12-08 14:26:44.000000000","message":"Thanks for the review guys!\n\nI have amended the patch according to your reviews.","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ba2a2be5f1c4bbc81f208b887c921c89b547f857","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"d3983833_07f780a2","updated":"2022-12-08 08:14:20.000000000","message":"The current implementation looks incorrect. The case you want to address is when uploading a volume to image which needs to filter out properties before sending metadata to glance here[1]. Also the backup change doesn\u0027t seem to be necessary unless it\u0027s addressing a specific case missed in the commit message, in that case, that should be mentioned in the commit message.\n\n[1] https://github.com/openstack/cinder/blob/24a783408879abd479e77dd88b43bf2a1be048fe/cinder/volume/api.py#L1486","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"d80dad7028bccf3ce34ebd5bdcad7c5a35c5fba4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"91237837_05201fb6","updated":"2022-11-28 13:00:12.000000000","message":"nevermind, it is patchset 21 that contains those changes, and the message for a new patchset uploaded in not the message list in gerrit.","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"7bd028f2d2b59032d1586f10cac049c4cf1550e0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"eb1a07c3_0f1efe7a","updated":"2022-11-28 13:01:31.000000000","message":"oh it is in the list once \"show all entries\" is selected. I really miss the \"hide all CI messages\" button","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":33807,"name":"Jacob Wang","email":"jacob_wang1@dell.com","username":"jacob0522"},"change_message_id":"2d91e577af53b229b82a11d51fa7fceedb6d8927","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"2df294e8_c00eb831","updated":"2022-11-25 07:25:52.000000000","message":"run-DellEMC PowerFlex CI","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"b3e23fb06c50cc27395e86612616dd6aadcc26cc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"757e21ca_1f9533d9","in_reply_to":"eb1a07c3_0f1efe7a","updated":"2022-11-28 13:19:49.000000000","message":"So, it is not just me 😊","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"916195168b4651fbbddf30465b12eb095747107e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"3ab147bf_b38f338c","updated":"2022-12-14 15:36:18.000000000","message":"Looks like comments have been addressed.  Should be good.","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b2f3426bfc7309e9bfe0ecce42721e4c8c664e23","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"ba825390_906fa04c","updated":"2022-12-20 17:22:24.000000000","message":"One issue noted in the testcase, and few changes in releasenote.","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"ae22d464c0f7428a48f181add1322202d1f0dfd2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"0f83a6a3_ca65d0fd","updated":"2022-12-21 11:26:43.000000000","message":"Thanks Rajat Dhasmana!\n\nI have amended the patch with your suggestions.","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"75a1e4dfd481805a8a1704ff9d4b7ca211319932","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"3c53e6ce_d8247ddd","updated":"2022-12-09 10:39:00.000000000","message":"recheck","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"b2fd9a28cd8394c185d97a7bf2e25ebb94d8f598","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"88d65e82_cd84f964","updated":"2022-12-08 20:43:15.000000000","message":"recheck","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"1af82ec61da0565455e9c4996253c8cc07798456","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"cba652ec_fc7f4e29","updated":"2022-12-09 21:15:13.000000000","message":"recheck","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"7f1a63f419c37003c303f99e2a8f001061c187a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"663792ae_b4ca3b71","updated":"2022-12-20 18:21:00.000000000","message":"run Pure Storage CI\n","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6b1d70dfcddfd8e60372b896b39a8206b44c0f05","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"3f5c33b7_a7a380b3","updated":"2022-12-21 18:02:42.000000000","message":"Fixed some minor things in releasenote. previously passed zuul so this time also it should.\nThanks Rafael for addressing all my comments. LGTM.","commit_id":"b8733451dbf6510376dd7120fc3c7fb3c69092c0"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"244b9a2bc951f7f549c28962da55d70dedbf17ec","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"7600fc5e_83ff51f2","updated":"2023-01-04 21:43:13.000000000","message":"Forgot to vote.","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fa925e6ffa5c6679115586578aa6bdad7154fc45","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"5a27ca9d_226f2042","updated":"2023-01-04 21:43:00.000000000","message":"I still need to review the tests.  In the meantime, the config opt help text is incorrect (suggested change noted inline).  Also, I think the release note could be made more clear (suggestions inline).","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"d979e8d65509b92ebdbc029c9ce362301df1c412","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"fc54a372_f0929d91","updated":"2023-01-05 16:33:00.000000000","message":"Thanks Brian for your review!","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"cda53259a528b1d061b72419dc3d6fb6f8f2e025","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"7f0d2eba_d117fed7","updated":"2022-12-22 11:50:20.000000000","message":"recheck","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"0eaf6d57ac3b8cdce3d5d81d057a8b0f0b641f87","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"4c872b1b_9a07640d","updated":"2023-01-06 00:47:59.000000000","message":"recheck","commit_id":"12f8ff5d89b7bdb41a101a070b32291f9f38a61a"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"bf7d3a317c3133fd1a70ce3b12a1fd7f8b3062aa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"5bbbf396_88de5415","updated":"2023-01-06 10:51:18.000000000","message":"recheck","commit_id":"12f8ff5d89b7bdb41a101a070b32291f9f38a61a"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"7858068e18ff3b566ba4ab670bf3e4f1c7f1b75c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"936cd08d_0f49ea32","updated":"2023-01-05 18:19:42.000000000","message":"recheck","commit_id":"12f8ff5d89b7bdb41a101a070b32291f9f38a61a"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"8c47a2b4e8128a3b14c3fb93cb974bd3148cdfff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"b0ac7bbb_0560e48b","updated":"2023-01-09 11:48:41.000000000","message":"recheck","commit_id":"12f8ff5d89b7bdb41a101a070b32291f9f38a61a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f4d5f19b766c06deaf419aaf03c0a5fecd54e5d8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"485374c3_2bbac36a","updated":"2023-01-18 13:42:05.000000000","message":"This looks really close.  The only reason I\u0027m -1\u0027ing is that the log debug message is misleading, and as something that\u0027s operator-visible, we can\u0027t let it slide.  (See comment inline.)  Otherwise, the code and tests and helpstrings/release notes LGTM.","commit_id":"60fb77d1bc5bc02bd4ee03faebfcb6cd05469e98"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"3d1f27c667fa50020358d868f4d8eb6b016fe35d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"4a5c1d46_7208b66d","updated":"2023-01-23 16:18:45.000000000","message":"I\u0027m really +2 on this change, except for the log message I\u0027ve flagged inline, where I\u0027d like to see what other reviewers think.  If it turns out I\u0027m the only person this bothers, I\u0027ll upgrade my vote to +2.","commit_id":"ab1b3cdd4c52b56debba6b15759ab910abc2226d"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"cf47ae1d218e3b0e93a5b2ff3599671632eed9c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"e05f143f_2d0bf279","updated":"2023-01-25 15:29:39.000000000","message":"needs rebase","commit_id":"ab1b3cdd4c52b56debba6b15759ab910abc2226d"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"9b4791209f7a6bbcdb3e4e137d41e44115cf9553","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"91e5f579_6613c98a","updated":"2023-01-25 16:01:50.000000000","message":"rebased","commit_id":"ab1b3cdd4c52b56debba6b15759ab910abc2226d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d39de6c8cc37ce02c1e63908310fe4719ce8e988","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"592d1819_e421b6cf","updated":"2023-01-30 06:39:34.000000000","message":"few nits but overall LGTM. Thanks Rafael for working on this!","commit_id":"9a4d1f279cf9376b95090ac7b7d0589ed7d8c7f1"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0691802bbfb0540c1de628babbb1aa93325ef69b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":39,"id":"2d8ec3b1_e8adde2a","updated":"2023-01-31 21:32:49.000000000","message":"LGTM.  Rafael and Rodrigo, thanks for all your work (and patience!) on this.","commit_id":"c1035b119f77735550be66653c97ff166900d062"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"95f9dd82619d197f2296174f28ab9d4ba3e8860e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":39,"id":"93413144_29c82716","updated":"2023-01-31 23:46:55.000000000","message":"recheck","commit_id":"c1035b119f77735550be66653c97ff166900d062"}],"cinder/api/contrib/volume_image_metadata.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a979a1d81a5619b7700ee80185d92a2d65c1c1b7","unresolved":true,"context_lines":[{"line_number":136,"context_line":"            key \u003d body[\u0027os-unset_image_metadata\u0027][\u0027key\u0027]"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"            vol, metadata \u003d self._get_image_metadata(context, id)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"            if key not in metadata:"},{"line_number":141,"context_line":"                raise exception.GlanceMetadataNotFound(id\u003did)"},{"line_number":142,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5767c65f_396a1294","line":139,"updated":"2022-06-22 17:12:22.000000000","message":"this can be removed.","commit_id":"c08104af0b4fb0cc7ca710e56d4e77c0fa643b19"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"bcd6c19cddd96da898442eeabc3c1f3f68e58615","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            key \u003d body[\u0027os-unset_image_metadata\u0027][\u0027key\u0027]"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"            vol, metadata \u003d self._get_image_metadata(context, id)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"            if key not in metadata:"},{"line_number":141,"context_line":"                raise exception.GlanceMetadataNotFound(id\u003did)"},{"line_number":142,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"f54cc1e4_98599cad","line":139,"in_reply_to":"5767c65f_396a1294","updated":"2022-06-22 17:42:57.000000000","message":"Done","commit_id":"c08104af0b4fb0cc7ca710e56d4e77c0fa643b19"}],"cinder/backup/driver.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ba2a2be5f1c4bbc81f208b887c921c89b547f857","unresolved":true,"context_lines":[{"line_number":260,"context_line":"                  volume_id, metadata)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        meta \u003d self._filter(metadata, fields)"},{"line_number":263,"context_line":"        meta \u003d image_utils.filter_out_reserved_namespaces_metadata(meta)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        LOG.debug(\"Metadata result [%s] after filtering, \""},{"line_number":266,"context_line":"                  \"original metadata [%s] for volume [%s].\","}],"source_content_type":"text/x-python","patch_set":23,"id":"324f003b_377cb94d","line":263,"range":{"start_line":263,"start_character":8,"end_line":263,"end_character":72},"updated":"2022-12-08 08:14:20.000000000","message":"here we are restoring the image metadata back to the volume during the restore operation[1].\nAs per the commit message, the problem we face with these glance reserved properties is when interacting with glance so we shouldn\u0027t be having any error case here.\nWith the current design, we keep the image properties in volume image metadata and only filter out when sending it to glance, my question is, is this change really necessary and which case does it address as we are not interacting with glance in this operation workflow?\n\n[1] https://github.com/openstack/cinder/blob/7086157de07b77e8b67bbb767bc2ce25e86c2f51/cinder/backup/chunkeddriver.py#L802","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"5522f3fe554b0adb619011227c161e04e0b1cb98","unresolved":false,"context_lines":[{"line_number":260,"context_line":"                  volume_id, metadata)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        meta \u003d self._filter(metadata, fields)"},{"line_number":263,"context_line":"        meta \u003d image_utils.filter_out_reserved_namespaces_metadata(meta)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        LOG.debug(\"Metadata result [%s] after filtering, \""},{"line_number":266,"context_line":"                  \"original metadata [%s] for volume [%s].\","}],"source_content_type":"text/x-python","patch_set":23,"id":"99920a04_5fa01bf5","line":263,"range":{"start_line":263,"start_character":8,"end_line":263,"end_character":72},"in_reply_to":"324f003b_377cb94d","updated":"2022-12-08 14:26:44.000000000","message":"Ack","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"}],"cinder/image/image_utils.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a979a1d81a5619b7700ee80185d92a2d65c1c1b7","unresolved":true,"context_lines":[{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":""},{"line_number":1145,"context_line":"def filter_out_reserved_namespaces_metadata(metadata):"},{"line_number":1146,"context_line":"    if not metadata:"},{"line_number":1147,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"},{"line_number":1148,"context_line":"        return {}"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"    if not isinstance(metadata, dict):"},{"line_number":1151,"context_line":"        LOG.info(\"Metadata object [%s] is not an instance of dict.\", metadata)"}],"source_content_type":"text/x-python","patch_set":10,"id":"9debe028_33584013","line":1148,"range":{"start_line":1146,"start_character":3,"end_line":1148,"end_character":17},"updated":"2022-06-22 17:12:22.000000000","message":"I think this should be checked before calling this method and no need to log a message for this.","commit_id":"c08104af0b4fb0cc7ca710e56d4e77c0fa643b19"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"bcd6c19cddd96da898442eeabc3c1f3f68e58615","unresolved":true,"context_lines":[{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":""},{"line_number":1145,"context_line":"def filter_out_reserved_namespaces_metadata(metadata):"},{"line_number":1146,"context_line":"    if not metadata:"},{"line_number":1147,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"},{"line_number":1148,"context_line":"        return {}"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"    if not isinstance(metadata, dict):"},{"line_number":1151,"context_line":"        LOG.info(\"Metadata object [%s] is not an instance of dict.\", metadata)"}],"source_content_type":"text/x-python","patch_set":10,"id":"c8143276_c817f116","line":1148,"range":{"start_line":1146,"start_character":3,"end_line":1148,"end_character":17},"in_reply_to":"9debe028_33584013","updated":"2022-06-22 17:42:57.000000000","message":"I see what you mean, but it was just a safer way of coding. I do not see much harm to maintain as is, and the log can help if we ever reach a situation when the metadata is empty.\n\nIf you strongly feel that this has to be removed, then I can work to remove it.","commit_id":"c08104af0b4fb0cc7ca710e56d4e77c0fa643b19"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"102e085b0d0865c383af4ddd35f220880f9a713a","unresolved":false,"context_lines":[{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":""},{"line_number":1145,"context_line":"def filter_out_reserved_namespaces_metadata(metadata):"},{"line_number":1146,"context_line":"    if not metadata:"},{"line_number":1147,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"},{"line_number":1148,"context_line":"        return {}"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"    if not isinstance(metadata, dict):"},{"line_number":1151,"context_line":"        LOG.info(\"Metadata object [%s] is not an instance of dict.\", metadata)"}],"source_content_type":"text/x-python","patch_set":10,"id":"cd0f80b9_562447d5","line":1148,"range":{"start_line":1146,"start_character":3,"end_line":1148,"end_character":17},"in_reply_to":"c8143276_c817f116","updated":"2022-06-29 11:51:14.000000000","message":"Done","commit_id":"c08104af0b4fb0cc7ca710e56d4e77c0fa643b19"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a979a1d81a5619b7700ee80185d92a2d65c1c1b7","unresolved":true,"context_lines":[{"line_number":1155,"context_line":"    for k, v in metadata.items():"},{"line_number":1156,"context_line":"        for reserved_name_space in CONF.reserved_image_namespaces:"},{"line_number":1157,"context_line":"            if k.startswith(reserved_name_space):"},{"line_number":1158,"context_line":"                LOG.debug(\"Ignoring key [%s] and value [%s] as the key is \""},{"line_number":1159,"context_line":"                          \"using a reserved namespace [%s].\","},{"line_number":1160,"context_line":"                          k, v, reserved_name_space)"},{"line_number":1161,"context_line":"                continue"},{"line_number":1162,"context_line":"            new_metadata[k] \u003d v"},{"line_number":1163,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"156d8c3d_d4a0d67d","line":1160,"range":{"start_line":1158,"start_character":16,"end_line":1160,"end_character":52},"updated":"2022-06-22 17:12:22.000000000","message":"\"Ignoring image property [%s] as it is reserved in glance\"?\n\nNo use of logging value for the property.","commit_id":"c08104af0b4fb0cc7ca710e56d4e77c0fa643b19"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"bcd6c19cddd96da898442eeabc3c1f3f68e58615","unresolved":true,"context_lines":[{"line_number":1155,"context_line":"    for k, v in metadata.items():"},{"line_number":1156,"context_line":"        for reserved_name_space in CONF.reserved_image_namespaces:"},{"line_number":1157,"context_line":"            if k.startswith(reserved_name_space):"},{"line_number":1158,"context_line":"                LOG.debug(\"Ignoring key [%s] and value [%s] as the key is \""},{"line_number":1159,"context_line":"                          \"using a reserved namespace [%s].\","},{"line_number":1160,"context_line":"                          k, v, reserved_name_space)"},{"line_number":1161,"context_line":"                continue"},{"line_number":1162,"context_line":"            new_metadata[k] \u003d v"},{"line_number":1163,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"cde0bc75_053da1db","line":1160,"range":{"start_line":1158,"start_character":16,"end_line":1160,"end_character":52},"in_reply_to":"156d8c3d_d4a0d67d","updated":"2022-06-22 17:42:57.000000000","message":"I actually see the value in logging the value of the key, and to show in the log message the configured reserved namespaces.","commit_id":"c08104af0b4fb0cc7ca710e56d4e77c0fa643b19"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"102e085b0d0865c383af4ddd35f220880f9a713a","unresolved":false,"context_lines":[{"line_number":1155,"context_line":"    for k, v in metadata.items():"},{"line_number":1156,"context_line":"        for reserved_name_space in CONF.reserved_image_namespaces:"},{"line_number":1157,"context_line":"            if k.startswith(reserved_name_space):"},{"line_number":1158,"context_line":"                LOG.debug(\"Ignoring key [%s] and value [%s] as the key is \""},{"line_number":1159,"context_line":"                          \"using a reserved namespace [%s].\","},{"line_number":1160,"context_line":"                          k, v, reserved_name_space)"},{"line_number":1161,"context_line":"                continue"},{"line_number":1162,"context_line":"            new_metadata[k] \u003d v"},{"line_number":1163,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7fcadfa6_085c0727","line":1160,"range":{"start_line":1158,"start_character":16,"end_line":1160,"end_character":52},"in_reply_to":"cde0bc75_053da1db","updated":"2022-06-29 11:51:14.000000000","message":"Done","commit_id":"c08104af0b4fb0cc7ca710e56d4e77c0fa643b19"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"307aac69f919ec311b0fc7924f0c6ccc4b89e403","unresolved":true,"context_lines":[{"line_number":1155,"context_line":"    for k, v in metadata.items():"},{"line_number":1156,"context_line":"        for reserved_name_space in CONF.reserved_image_namespaces:"},{"line_number":1157,"context_line":"            if k.startswith(reserved_name_space):"},{"line_number":1158,"context_line":"                LOG.debug(\"Ignoring key [%s] and value [%s] as the key is \""},{"line_number":1159,"context_line":"                          \"using a reserved namespace [%s].\","},{"line_number":1160,"context_line":"                          k, v, reserved_name_space)"},{"line_number":1161,"context_line":"                continue"},{"line_number":1162,"context_line":"            new_metadata[k] \u003d v"},{"line_number":1163,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"c5667d44_8452010f","line":1160,"range":{"start_line":1158,"start_character":16,"end_line":1160,"end_character":52},"updated":"2022-11-02 14:43:59.000000000","message":"This doesn\u0027t seem like a good place to put a DEBUG message. If we have a lot of metadata items and reserved namespaces, this will flood the logs.\n\nIMO the LOG on L#1164 is sufficient to know which all properties were skipped and we don\u0027t need this.","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"347fad509963aa776c906000383ccd696bdece39","unresolved":false,"context_lines":[{"line_number":1155,"context_line":"    for k, v in metadata.items():"},{"line_number":1156,"context_line":"        for reserved_name_space in CONF.reserved_image_namespaces:"},{"line_number":1157,"context_line":"            if k.startswith(reserved_name_space):"},{"line_number":1158,"context_line":"                LOG.debug(\"Ignoring key [%s] and value [%s] as the key is \""},{"line_number":1159,"context_line":"                          \"using a reserved namespace [%s].\","},{"line_number":1160,"context_line":"                          k, v, reserved_name_space)"},{"line_number":1161,"context_line":"                continue"},{"line_number":1162,"context_line":"            new_metadata[k] \u003d v"},{"line_number":1163,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"0e963939_3214aaa5","line":1160,"range":{"start_line":1158,"start_character":16,"end_line":1160,"end_character":52},"in_reply_to":"c5667d44_8452010f","updated":"2022-11-08 16:18:09.000000000","message":"Done","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"38216a023f1a4acd544376673935549f80b723db","unresolved":true,"context_lines":[{"line_number":86,"context_line":"                     \u0027database. The namespaces informed will be used to \u0027"},{"line_number":87,"context_line":"                     \u0027remove/strip/filter the metadata that start with it. \u0027"},{"line_number":88,"context_line":"                     \u0027The default value is `[\"os_glance\"]`.\u0027,"},{"line_number":89,"context_line":"                default\u003d[\"os_glance\"]),"},{"line_number":90,"context_line":"]"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":18,"id":"8f71bc92_d1845e30","line":89,"updated":"2022-11-10 14:08:18.000000000","message":"A few questions about this.\n\n- Why filter on download?  Are we sure that there\u0027s no utility to having these properties as part of the volume image metadata?  Since the problem is that Glance won\u0027t let you create an image with these properties, why not filter them out on upload as Nova does?\n\n- Note that Nova didn\u0027t set a config opt for this, it\u0027s hard-coded, and in fact, Nova added a hard-coded list of absolutely-non-inheritable-image-properties in addition to Nova\u0027s legacy noninheritable_image_properties config opt.  The idea was to avoid mis-configuration of the opt that could lead to problems.  I don\u0027t object to this config opt (I think it could be useful), but I wonder whether we should also have the hard-coded list too?","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e3a23c7eb36cf8587f83ecd767bcac517541c4cb","unresolved":true,"context_lines":[{"line_number":86,"context_line":"                     \u0027database. The namespaces informed will be used to \u0027"},{"line_number":87,"context_line":"                     \u0027remove/strip/filter the metadata that start with it. \u0027"},{"line_number":88,"context_line":"                     \u0027The default value is `[\"os_glance\"]`.\u0027,"},{"line_number":89,"context_line":"                default\u003d[\"os_glance\"]),"},{"line_number":90,"context_line":"]"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":18,"id":"b5ab8f7b_f18e5983","line":89,"in_reply_to":"5bc69e90_7ef997a0","updated":"2022-11-15 14:41:42.000000000","message":"I agree that the config option is definitely needed.  My question is whether we should *additionally* have the hard-coded list (to keep the similarity to the nova solution!).  I don\u0027t have a strong opinion on this; the issue is that Glance will *never* allow users to set properties in the \u0027os_glance\u0027 namespace, so if operators remove it by mistake when setting the config option, they will break all upload-volume-to-image calls.  Which they should notice and fix fairly quickly, but we could avoid this situation entirely with the additional hard-coded list.  Or we could simply point out in the help text for the option that you should only add stuff to the list, you shouldn\u0027t remove anything.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2993b8aafaeef5c1443701995168de839bb3f8bb","unresolved":true,"context_lines":[{"line_number":86,"context_line":"                     \u0027database. The namespaces informed will be used to \u0027"},{"line_number":87,"context_line":"                     \u0027remove/strip/filter the metadata that start with it. \u0027"},{"line_number":88,"context_line":"                     \u0027The default value is `[\"os_glance\"]`.\u0027,"},{"line_number":89,"context_line":"                default\u003d[\"os_glance\"]),"},{"line_number":90,"context_line":"]"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":18,"id":"efd76a43_0534ce31","line":89,"in_reply_to":"8f71bc92_d1845e30","updated":"2022-11-10 20:01:27.000000000","message":"1) I am personally unaware of any situation other than upload-back-to-glance where this causes problems, despite the release notes stating that the metadata could cause problems on several interactions with glance. But indeed if only the upload-back-to-glance causes problems, I see no benefit in adding extra code to not save the metadata, aside from the fact that the metadata in question is generally useless to cinder.\n\n2) I personally prefer having the config option because it allows the behavior to be configurable, it is not clear to me the need to add more hardcoded filter properties.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"0b137a1e0c0d0424eb6aa0da864c704d896d9107","unresolved":true,"context_lines":[{"line_number":86,"context_line":"                     \u0027database. The namespaces informed will be used to \u0027"},{"line_number":87,"context_line":"                     \u0027remove/strip/filter the metadata that start with it. \u0027"},{"line_number":88,"context_line":"                     \u0027The default value is `[\"os_glance\"]`.\u0027,"},{"line_number":89,"context_line":"                default\u003d[\"os_glance\"]),"},{"line_number":90,"context_line":"]"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":18,"id":"dd087387_02fb284d","line":89,"in_reply_to":"b5ab8f7b_f18e5983","updated":"2022-11-15 17:33:27.000000000","message":"+1 for help text that os_glance shouldn\u0027t be removed","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"25fbbe4bfbe0554b5e5237e8a0e00f3841fdd4ea","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                     \u0027database. The namespaces informed will be used to \u0027"},{"line_number":87,"context_line":"                     \u0027remove/strip/filter the metadata that start with it. \u0027"},{"line_number":88,"context_line":"                     \u0027The default value is `[\"os_glance\"]`.\u0027,"},{"line_number":89,"context_line":"                default\u003d[\"os_glance\"]),"},{"line_number":90,"context_line":"]"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":18,"id":"ecd9125b_52873422","line":89,"in_reply_to":"b5ab8f7b_f18e5983","updated":"2022-11-24 14:52:01.000000000","message":"Done","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"17694ac4b4d8431228c5ab219656651d38123391","unresolved":true,"context_lines":[{"line_number":86,"context_line":"                     \u0027database. The namespaces informed will be used to \u0027"},{"line_number":87,"context_line":"                     \u0027remove/strip/filter the metadata that start with it. \u0027"},{"line_number":88,"context_line":"                     \u0027The default value is `[\"os_glance\"]`.\u0027,"},{"line_number":89,"context_line":"                default\u003d[\"os_glance\"]),"},{"line_number":90,"context_line":"]"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":18,"id":"5bc69e90_7ef997a0","line":89,"in_reply_to":"efd76a43_0534ce31","updated":"2022-11-11 13:48:24.000000000","message":"We can hard-code it, if you prefer. I actually prefer it. I only externalized to an option due to a comment on \"Oct 07, 2021 9:00 AM\", more precisely the comment:\n```\nWe need the config option (not just the hard-coded list) because glance property protections leave the possibility that an operator has configured read-only image properties in glance, and if so, they can use the option in cinder so that cinder doesn\u0027t try to write them when creating an image in glance.\n```\n\nEnglish is not my mother language; therefore, I might have misunderstood it. That is why I created this option. Any alternative is fine by me. You guys just need to decide what you prefer.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ec62702b0e248cafc738c3a5fd93d57c83a8a65b","unresolved":true,"context_lines":[{"line_number":1147,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"},{"line_number":1148,"context_line":"        return {}"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"    if not isinstance(metadata, dict):"},{"line_number":1151,"context_line":"        LOG.info(\"Metadata object [%s] is not an instance of dict.\", metadata)"},{"line_number":1152,"context_line":"        return metadata"},{"line_number":1153,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"8ff2bedc_51f6de2b","line":1150,"range":{"start_line":1150,"start_character":4,"end_line":1150,"end_character":38},"updated":"2022-11-09 23:44:51.000000000","message":"Does this actually happen somewhere in the code, or are you being proactive?  If you want to make sure you get a dict, you can use type checking to prevent someone from passing something else when you declare the parameter, e.g.,\n\n  def filter_out(metadata: Optional[dict[str, str]]):\n\nand in that case, you\u0027ll always be returning a dict, so you can annotate that as well:\n\n  def filter_out(metadata: Optional[dict[str, str]]) -\u003e dict[str, str]:\n\nIf you do need this check, though, I wonder if the log level should be debug.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e3a23c7eb36cf8587f83ecd767bcac517541c4cb","unresolved":true,"context_lines":[{"line_number":1147,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"},{"line_number":1148,"context_line":"        return {}"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"    if not isinstance(metadata, dict):"},{"line_number":1151,"context_line":"        LOG.info(\"Metadata object [%s] is not an instance of dict.\", metadata)"},{"line_number":1152,"context_line":"        return metadata"},{"line_number":1153,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"6e6c63a6_cae4cca0","line":1150,"range":{"start_line":1150,"start_character":4,"end_line":1150,"end_character":38},"in_reply_to":"568e36f5_4fc07ba2","updated":"2022-11-15 14:41:42.000000000","message":"I know this sucks, but I think we need to use the type annotations and fix whatever tests break.\n\nThat will make the log message irrelevant because it won\u0027t be necessary any more.\n\nOn the plus side, when I comment out lines 1150-1152, some of the failing tests are in the BackupMetadata API, and on the theory that we\u0027re only going to filter these out on upload to Glance, you\u0027ll be removing the changes to the backup service, so you won\u0027t have to worry about those tests.  Another failing test is the one that you wrote specifically to check this, so you won\u0027t need that test any more.  There\u0027s one other test that broke in the rbd driver, but I\u0027m not sure what\u0027s going on there (and it may be that moving the filtering to upload only will make that go away too).","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"25fbbe4bfbe0554b5e5237e8a0e00f3841fdd4ea","unresolved":false,"context_lines":[{"line_number":1147,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"},{"line_number":1148,"context_line":"        return {}"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"    if not isinstance(metadata, dict):"},{"line_number":1151,"context_line":"        LOG.info(\"Metadata object [%s] is not an instance of dict.\", metadata)"},{"line_number":1152,"context_line":"        return metadata"},{"line_number":1153,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"b37cedb9_681d9c93","line":1150,"range":{"start_line":1150,"start_character":4,"end_line":1150,"end_character":38},"in_reply_to":"6e6c63a6_cae4cca0","updated":"2022-11-24 14:52:01.000000000","message":"Done","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"17694ac4b4d8431228c5ab219656651d38123391","unresolved":true,"context_lines":[{"line_number":1147,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"},{"line_number":1148,"context_line":"        return {}"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"    if not isinstance(metadata, dict):"},{"line_number":1151,"context_line":"        LOG.info(\"Metadata object [%s] is not an instance of dict.\", metadata)"},{"line_number":1152,"context_line":"        return metadata"},{"line_number":1153,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"568e36f5_4fc07ba2","line":1150,"range":{"start_line":1150,"start_character":4,"end_line":1150,"end_character":38},"in_reply_to":"7073b025_168a0b47","updated":"2022-11-11 13:48:24.000000000","message":"It has been so long since I create the patch, and it being considered for review (more than an year) that I do not remember anymore. However, I think that there are some tests that use mock objects that were breaking this code, and that is why I added this condition here.\n\nIt is not to avoid coding issue, but to deal with some situation in the tests.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2993b8aafaeef5c1443701995168de839bb3f8bb","unresolved":true,"context_lines":[{"line_number":1147,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"},{"line_number":1148,"context_line":"        return {}"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"    if not isinstance(metadata, dict):"},{"line_number":1151,"context_line":"        LOG.info(\"Metadata object [%s] is not an instance of dict.\", metadata)"},{"line_number":1152,"context_line":"        return metadata"},{"line_number":1153,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"7073b025_168a0b47","line":1150,"range":{"start_line":1150,"start_character":4,"end_line":1150,"end_character":38},"in_reply_to":"8ff2bedc_51f6de2b","updated":"2022-11-10 20:01:27.000000000","message":"hmmm I tested this in a python3 shell (I don\u0027t normally do any typing annotations) and I don\u0027t see any benefit in behavior, just reduces the amount of code type guessing when reading the code. Here I see the advantage of a log message, that I agree should not be info, perhaps it should be warning IMO, not debug, as it is improper behavior we don\u0027t want coded. I\u0027m waiting to hear more on this before making changes (warnings vs debug, typing annotations agreement)","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2edc4b7d56f9450afffcbeb4cd9fe2b36e018960","unresolved":true,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"COMPRESSIBLE_IMAGE_FORMATS \u003d (\u0027qcow2\u0027,)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"GLANCE_RESERVED_NAMESPACE \u003d [\"os_glance\"]"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def validate_stores_id(context: context.RequestContext,"}],"source_content_type":"text/x-python","patch_set":23,"id":"85c0827f_6dbdc6cd","line":115,"range":{"start_line":115,"start_character":0,"end_line":115,"end_character":41},"updated":"2022-12-07 14:38:28.000000000","message":"I suggest making this plural:\n\n  GLANCE_RESERVED_NAMESPACES \u003d [\u0027os_glance\u0027, \u0027img_signature\u0027]","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"5522f3fe554b0adb619011227c161e04e0b1cb98","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"COMPRESSIBLE_IMAGE_FORMATS \u003d (\u0027qcow2\u0027,)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"GLANCE_RESERVED_NAMESPACE \u003d [\"os_glance\"]"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def validate_stores_id(context: context.RequestContext,"}],"source_content_type":"text/x-python","patch_set":23,"id":"3710a6d9_6fd0bf44","line":115,"range":{"start_line":115,"start_character":0,"end_line":115,"end_character":41},"in_reply_to":"85c0827f_6dbdc6cd","updated":"2022-12-08 14:26:44.000000000","message":"Done","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fa925e6ffa5c6679115586578aa6bdad7154fc45","unresolved":true,"context_lines":[{"line_number":81,"context_line":"                \u0027can cause performance problems on the cinder-volume node. \u0027"},{"line_number":82,"context_line":"                \u0027When set True, this option disables image conversion.\u0027),"},{"line_number":83,"context_line":"    cfg.ListOpt(\u0027reserved_image_namespaces\u0027,"},{"line_number":84,"context_line":"                help\u003d\u0027List of reserved image namespaces that should be \u0027"},{"line_number":85,"context_line":"                     \u0027filtered out when Cinder stores image metadata in its \u0027"},{"line_number":86,"context_line":"                     \u0027database. The namespaces informed will be used to \u0027"},{"line_number":87,"context_line":"                     \u0027remove/strip/filter the metadata that start with it. \u0027"},{"line_number":88,"context_line":"                     \u0027The default value is `[]`. Moreover, we always add \u0027"},{"line_number":89,"context_line":"                     \u0027`os_glance` as a reserved namespace.\u0027,"},{"line_number":90,"context_line":"                default\u003d[]),"},{"line_number":91,"context_line":"]"},{"line_number":92,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"730c387d_b6a76a62","line":89,"range":{"start_line":84,"start_character":22,"end_line":89,"end_character":60},"updated":"2023-01-04 21:43:00.000000000","message":"I think this is inaccurate in a few ways.  Maybe something more like this:\n\n  List of reserved image namespaces that should be filtered out when uploading\n  a volume as an image.  When a volume is created from an image, cinder stores\n  the image properties as volume image_metadata, and if that volume is later\n  uploaded as a image, cinder will add these properties when it creates the image\n  in glance.  This can cause problems for image_metadata that are in namespaces\n  that glance reserves for itself, or when properties (such as an image\n  signature) cannot apply to the new image, or when an operator has configured\n  glance property protections to make some image properties read-only.  Cinder\n  will *always* filter out image_metadata in the namespaces \u0027os_glance\u0027 and\n  \u0027img_signature\u0027; this configuration option allows you to specify *additional*\n  namespaces to be excluded.","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"d979e8d65509b92ebdbc029c9ce362301df1c412","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                \u0027can cause performance problems on the cinder-volume node. \u0027"},{"line_number":82,"context_line":"                \u0027When set True, this option disables image conversion.\u0027),"},{"line_number":83,"context_line":"    cfg.ListOpt(\u0027reserved_image_namespaces\u0027,"},{"line_number":84,"context_line":"                help\u003d\u0027List of reserved image namespaces that should be \u0027"},{"line_number":85,"context_line":"                     \u0027filtered out when Cinder stores image metadata in its \u0027"},{"line_number":86,"context_line":"                     \u0027database. The namespaces informed will be used to \u0027"},{"line_number":87,"context_line":"                     \u0027remove/strip/filter the metadata that start with it. \u0027"},{"line_number":88,"context_line":"                     \u0027The default value is `[]`. Moreover, we always add \u0027"},{"line_number":89,"context_line":"                     \u0027`os_glance` as a reserved namespace.\u0027,"},{"line_number":90,"context_line":"                default\u003d[]),"},{"line_number":91,"context_line":"]"},{"line_number":92,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"db070e9b_1f229cc8","line":89,"range":{"start_line":84,"start_character":22,"end_line":89,"end_character":60},"in_reply_to":"730c387d_b6a76a62","updated":"2023-01-05 16:33:00.000000000","message":"Ack","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fa925e6ffa5c6679115586578aa6bdad7154fc45","unresolved":true,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"COMPRESSIBLE_IMAGE_FORMATS \u003d (\u0027qcow2\u0027,)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"GLANCE_RESERVED_NAMESPACES \u003d [\"os_glance\", \u0027img_signature\u0027]"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def validate_stores_id(context: context.RequestContext,"}],"source_content_type":"text/x-python","patch_set":32,"id":"74107791_102a657f","line":115,"range":{"start_line":115,"start_character":30,"end_line":115,"end_character":41},"updated":"2023-01-04 21:43:00.000000000","message":"super-nit: it\u0027s nice to be consistent with which kind of quotes you\u0027re using; the single quote will work fine here.","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"d979e8d65509b92ebdbc029c9ce362301df1c412","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"COMPRESSIBLE_IMAGE_FORMATS \u003d (\u0027qcow2\u0027,)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"GLANCE_RESERVED_NAMESPACES \u003d [\"os_glance\", \u0027img_signature\u0027]"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def validate_stores_id(context: context.RequestContext,"}],"source_content_type":"text/x-python","patch_set":32,"id":"30a94933_62ec949b","line":115,"range":{"start_line":115,"start_character":30,"end_line":115,"end_character":41},"in_reply_to":"74107791_102a657f","updated":"2023-01-05 16:33:00.000000000","message":"Done","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fa925e6ffa5c6679115586578aa6bdad7154fc45","unresolved":true,"context_lines":[{"line_number":1149,"context_line":"        metadata: Optional[dict[str, str]]) -\u003e dict[str, str]:"},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"    reserved_name_spaces \u003d\\"},{"line_number":1152,"context_line":"        GLANCE_RESERVED_NAMESPACES + CONF.reserved_image_namespaces"},{"line_number":1153,"context_line":""},{"line_number":1154,"context_line":"    if not metadata:"},{"line_number":1155,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"}],"source_content_type":"text/x-python","patch_set":32,"id":"6ca73462_ffcaac4d","line":1152,"updated":"2023-01-04 21:43:00.000000000","message":"If an operator includes a hard-coded namespace in the config option, it looks like it will just be checked twice in the loop, which is probably OK.  (I don\u0027t think it would cause an error, but just wanted to flag this.  Maybe you already have a unit test covering this situation (I haven\u0027t reviewed that file yet).","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"d979e8d65509b92ebdbc029c9ce362301df1c412","unresolved":false,"context_lines":[{"line_number":1149,"context_line":"        metadata: Optional[dict[str, str]]) -\u003e dict[str, str]:"},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"    reserved_name_spaces \u003d\\"},{"line_number":1152,"context_line":"        GLANCE_RESERVED_NAMESPACES + CONF.reserved_image_namespaces"},{"line_number":1153,"context_line":""},{"line_number":1154,"context_line":"    if not metadata:"},{"line_number":1155,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"}],"source_content_type":"text/x-python","patch_set":32,"id":"f32430e3_0147d9ba","line":1152,"in_reply_to":"6ca73462_ffcaac4d","updated":"2023-01-05 16:33:00.000000000","message":"I am checking the namespaces before adding them to the list then","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f4d5f19b766c06deaf419aaf03c0a5fecd54e5d8","unresolved":true,"context_lines":[{"line_number":1163,"context_line":"        for image_namespace in CONF.reserved_image_namespaces:"},{"line_number":1164,"context_line":"            if image_namespace not in reserved_name_spaces:"},{"line_number":1165,"context_line":"                reserved_name_spaces.append(image_namespace)"},{"line_number":1166,"context_line":"            else:"},{"line_number":1167,"context_line":"                LOG.debug(\"Ignoring namespace [%s] as it is configured more \""},{"line_number":1168,"context_line":"                          \"than one time.\", image_namespace)"},{"line_number":1169,"context_line":""},{"line_number":1170,"context_line":"    if not metadata:"},{"line_number":1171,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"}],"source_content_type":"text/x-python","patch_set":34,"id":"18a605b2_d81e53c8","line":1168,"range":{"start_line":1166,"start_character":0,"end_line":1168,"end_character":60},"updated":"2023-01-18 13:42:05.000000000","message":"Since this won\u0027t hurt anything, I\u0027m not sure that you need to log it.  See what other reviewers think, but I recommend removing it, especially because:\n\n-1: The problem with this message is that it\u0027s misleading.  You\u0027re not ignoring the namespace completely, you\u0027re just ignoring the fact that it\u0027s configured twice.","commit_id":"60fb77d1bc5bc02bd4ee03faebfcb6cd05469e98"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"6a9a31358079cdcd48fff4c3c1fc1376645ec6c9","unresolved":true,"context_lines":[{"line_number":1163,"context_line":"        for image_namespace in CONF.reserved_image_namespaces:"},{"line_number":1164,"context_line":"            if image_namespace not in reserved_name_spaces:"},{"line_number":1165,"context_line":"                reserved_name_spaces.append(image_namespace)"},{"line_number":1166,"context_line":"            else:"},{"line_number":1167,"context_line":"                LOG.debug(\"Ignoring namespace [%s] as it is configured more \""},{"line_number":1168,"context_line":"                          \"than one time.\", image_namespace)"},{"line_number":1169,"context_line":""},{"line_number":1170,"context_line":"    if not metadata:"},{"line_number":1171,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"}],"source_content_type":"text/x-python","patch_set":34,"id":"220cc1da_7005193f","line":1168,"range":{"start_line":1166,"start_character":0,"end_line":1168,"end_character":60},"in_reply_to":"18a605b2_d81e53c8","updated":"2023-01-20 20:37:49.000000000","message":"I see what you mean. I amended the logs. What do you think now?","commit_id":"60fb77d1bc5bc02bd4ee03faebfcb6cd05469e98"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"66eba4d9d9bfbdf9cbf0334f3286d2364f87482d","unresolved":true,"context_lines":[{"line_number":1163,"context_line":"        for image_namespace in CONF.reserved_image_namespaces:"},{"line_number":1164,"context_line":"            if image_namespace not in reserved_name_spaces:"},{"line_number":1165,"context_line":"                reserved_name_spaces.append(image_namespace)"},{"line_number":1166,"context_line":"            else:"},{"line_number":1167,"context_line":"                LOG.debug(\"Ignoring namespace [%s] as it is configured more \""},{"line_number":1168,"context_line":"                          \"than one time.\", image_namespace)"},{"line_number":1169,"context_line":""},{"line_number":1170,"context_line":"    if not metadata:"},{"line_number":1171,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"}],"source_content_type":"text/x-python","patch_set":34,"id":"905f9d2d_f91d4ac9","line":1168,"range":{"start_line":1166,"start_character":0,"end_line":1168,"end_character":60},"in_reply_to":"220cc1da_7005193f","updated":"2023-01-20 21:21:42.000000000","message":"Definitely better, though I\u0027m not sold on logging this at all.  Let\u0027s see what other people think.","commit_id":"60fb77d1bc5bc02bd4ee03faebfcb6cd05469e98"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0691802bbfb0540c1de628babbb1aa93325ef69b","unresolved":false,"context_lines":[{"line_number":1163,"context_line":"        for image_namespace in CONF.reserved_image_namespaces:"},{"line_number":1164,"context_line":"            if image_namespace not in reserved_name_spaces:"},{"line_number":1165,"context_line":"                reserved_name_spaces.append(image_namespace)"},{"line_number":1166,"context_line":"            else:"},{"line_number":1167,"context_line":"                LOG.debug(\"Ignoring namespace [%s] as it is configured more \""},{"line_number":1168,"context_line":"                          \"than one time.\", image_namespace)"},{"line_number":1169,"context_line":""},{"line_number":1170,"context_line":"    if not metadata:"},{"line_number":1171,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"}],"source_content_type":"text/x-python","patch_set":34,"id":"51bb3dbc_9cdbad13","line":1168,"range":{"start_line":1166,"start_character":0,"end_line":1168,"end_character":60},"in_reply_to":"905f9d2d_f91d4ac9","updated":"2023-01-31 21:32:49.000000000","message":"Done","commit_id":"60fb77d1bc5bc02bd4ee03faebfcb6cd05469e98"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"66eba4d9d9bfbdf9cbf0334f3286d2364f87482d","unresolved":true,"context_lines":[{"line_number":1164,"context_line":"            if image_namespace not in reserved_name_spaces:"},{"line_number":1165,"context_line":"                reserved_name_spaces.append(image_namespace)"},{"line_number":1166,"context_line":"            else:"},{"line_number":1167,"context_line":"                LOG.debug(\"Namespace [%s] is configured more than one time.\""},{"line_number":1168,"context_line":"                          \"Please consider amending the configuration.\","},{"line_number":1169,"context_line":"                          image_namespace)"},{"line_number":1170,"context_line":""}],"source_content_type":"text/x-python","patch_set":35,"id":"6d3ae0a2_23dc8213","line":1167,"range":{"start_line":1167,"start_character":74,"end_line":1167,"end_character":75},"updated":"2023-01-20 21:21:42.000000000","message":"need a space after this period","commit_id":"432899f9681dfa7ea6b8b3fede8898a097bba214"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0691802bbfb0540c1de628babbb1aa93325ef69b","unresolved":false,"context_lines":[{"line_number":1164,"context_line":"            if image_namespace not in reserved_name_spaces:"},{"line_number":1165,"context_line":"                reserved_name_spaces.append(image_namespace)"},{"line_number":1166,"context_line":"            else:"},{"line_number":1167,"context_line":"                LOG.debug(\"Namespace [%s] is configured more than one time.\""},{"line_number":1168,"context_line":"                          \"Please consider amending the configuration.\","},{"line_number":1169,"context_line":"                          image_namespace)"},{"line_number":1170,"context_line":""}],"source_content_type":"text/x-python","patch_set":35,"id":"cd28197f_3c687cac","line":1167,"range":{"start_line":1167,"start_character":74,"end_line":1167,"end_character":75},"in_reply_to":"6d3ae0a2_23dc8213","updated":"2023-01-31 21:32:49.000000000","message":"Done","commit_id":"432899f9681dfa7ea6b8b3fede8898a097bba214"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"3d1f27c667fa50020358d868f4d8eb6b016fe35d","unresolved":true,"context_lines":[{"line_number":1164,"context_line":"            if image_namespace not in reserved_name_spaces:"},{"line_number":1165,"context_line":"                reserved_name_spaces.append(image_namespace)"},{"line_number":1166,"context_line":"            else:"},{"line_number":1167,"context_line":"                LOG.debug(\"Namespace [%s] is configured more than one time. \""},{"line_number":1168,"context_line":"                          \"Please consider amending the configuration.\","},{"line_number":1169,"context_line":"                          image_namespace)"},{"line_number":1170,"context_line":""},{"line_number":1171,"context_line":"    if not metadata:"},{"line_number":1172,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"}],"source_content_type":"text/x-python","patch_set":36,"id":"aee5c827_6574b335","line":1169,"range":{"start_line":1167,"start_character":0,"end_line":1169,"end_character":42},"updated":"2023-01-23 16:18:45.000000000","message":"I continue to think that we should just silently ignore multiple configuration.  Your code already makes sure that a string from the config is only included once in reserved_name_spaces, so it\u0027s not like we\u0027re going to have to debug an issue where reserved_name_spaces contains duplicate entries.  Additionally, we have the problem that if an operator adds \"os_glance\" to the config option, this message will be logged, but when they go to fix it, they will only see one \u0027os_glance\u0027 entry in the config list.\n\nAnyway, just flagging this to get more input from other reviewers.","commit_id":"ab1b3cdd4c52b56debba6b15759ab910abc2226d"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"0af7bd649a9ee179d420dc1e716f5f3d03c6b6f0","unresolved":false,"context_lines":[{"line_number":1164,"context_line":"            if image_namespace not in reserved_name_spaces:"},{"line_number":1165,"context_line":"                reserved_name_spaces.append(image_namespace)"},{"line_number":1166,"context_line":"            else:"},{"line_number":1167,"context_line":"                LOG.debug(\"Namespace [%s] is configured more than one time. \""},{"line_number":1168,"context_line":"                          \"Please consider amending the configuration.\","},{"line_number":1169,"context_line":"                          image_namespace)"},{"line_number":1170,"context_line":""},{"line_number":1171,"context_line":"    if not metadata:"},{"line_number":1172,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"}],"source_content_type":"text/x-python","patch_set":36,"id":"3b6c0aa4_5b02efaf","line":1169,"range":{"start_line":1167,"start_character":0,"end_line":1169,"end_character":42},"in_reply_to":"ab871f02_20392cc7","updated":"2023-01-27 18:02:53.000000000","message":"Ack","commit_id":"ab1b3cdd4c52b56debba6b15759ab910abc2226d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4b383187a4251b100a7d0a90d35201fc238af4b5","unresolved":true,"context_lines":[{"line_number":1164,"context_line":"            if image_namespace not in reserved_name_spaces:"},{"line_number":1165,"context_line":"                reserved_name_spaces.append(image_namespace)"},{"line_number":1166,"context_line":"            else:"},{"line_number":1167,"context_line":"                LOG.debug(\"Namespace [%s] is configured more than one time. \""},{"line_number":1168,"context_line":"                          \"Please consider amending the configuration.\","},{"line_number":1169,"context_line":"                          image_namespace)"},{"line_number":1170,"context_line":""},{"line_number":1171,"context_line":"    if not metadata:"},{"line_number":1172,"context_line":"        LOG.debug(\"No metadata to be filtered.\")"}],"source_content_type":"text/x-python","patch_set":36,"id":"ab871f02_20392cc7","line":1169,"range":{"start_line":1167,"start_character":0,"end_line":1169,"end_character":42},"in_reply_to":"aee5c827_6574b335","updated":"2023-01-27 14:39:40.000000000","message":"I agree with Brian here. AFAIU this message doesn\u0027t serve a purpose in debugging any issue, it will just tell the operator that the same namespace is configured twice (once by cinder and once by operator) which does not have any functional implications.\nAlso I don\u0027t think operators should be configuring \"os_glance\" or \"img_signature\" namespaces since the releasenote clearly mentions they\u0027re going to be excluded always.\n\n(the `os_glance` and `img_signature` namespaces are *always* excluded).","commit_id":"ab1b3cdd4c52b56debba6b15759ab910abc2226d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d39de6c8cc37ce02c1e63908310fe4719ce8e988","unresolved":true,"context_lines":[{"line_number":93,"context_line":"                help\u003d\u0027List of reserved image namespaces that should be \u0027"},{"line_number":94,"context_line":"                     \u0027filtered out when uploading a volume as an image back \u0027"},{"line_number":95,"context_line":"                     \u0027to Glance. When a volume is created from an image, \u0027"},{"line_number":96,"context_line":"                     \u0027Cinder stores the image properties as volume \u0027"},{"line_number":97,"context_line":"                     \u0027image_metadata, and if the volume is later uploaded as \u0027"},{"line_number":98,"context_line":"                     \u0027a image, Cinder will add these properties when it \u0027"},{"line_number":99,"context_line":"                     \u0027creates the image in Glance. This can cause problems \u0027"},{"line_number":100,"context_line":"                     \u0027for image metadata that are in namespaces that glance \u0027"}],"source_content_type":"text/x-python","patch_set":38,"id":"931c3303_7d297d8d","line":97,"range":{"start_line":96,"start_character":60,"end_line":97,"end_character":36},"updated":"2023-01-30 06:39:34.000000000","message":"nit: volume image metadata or volume\u0027s image_metadata or volume_image_metadata","commit_id":"9a4d1f279cf9376b95090ac7b7d0589ed7d8c7f1"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"009d3843227e8c6bb910a8ed32ca65b4be688bda","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                help\u003d\u0027List of reserved image namespaces that should be \u0027"},{"line_number":94,"context_line":"                     \u0027filtered out when uploading a volume as an image back \u0027"},{"line_number":95,"context_line":"                     \u0027to Glance. When a volume is created from an image, \u0027"},{"line_number":96,"context_line":"                     \u0027Cinder stores the image properties as volume \u0027"},{"line_number":97,"context_line":"                     \u0027image_metadata, and if the volume is later uploaded as \u0027"},{"line_number":98,"context_line":"                     \u0027a image, Cinder will add these properties when it \u0027"},{"line_number":99,"context_line":"                     \u0027creates the image in Glance. This can cause problems \u0027"},{"line_number":100,"context_line":"                     \u0027for image metadata that are in namespaces that glance \u0027"}],"source_content_type":"text/x-python","patch_set":38,"id":"21eb9daa_61df83fd","line":97,"range":{"start_line":96,"start_character":60,"end_line":97,"end_character":36},"in_reply_to":"931c3303_7d297d8d","updated":"2023-01-30 11:45:53.000000000","message":"Done","commit_id":"9a4d1f279cf9376b95090ac7b7d0589ed7d8c7f1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d39de6c8cc37ce02c1e63908310fe4719ce8e988","unresolved":true,"context_lines":[{"line_number":95,"context_line":"                     \u0027to Glance. When a volume is created from an image, \u0027"},{"line_number":96,"context_line":"                     \u0027Cinder stores the image properties as volume \u0027"},{"line_number":97,"context_line":"                     \u0027image_metadata, and if the volume is later uploaded as \u0027"},{"line_number":98,"context_line":"                     \u0027a image, Cinder will add these properties when it \u0027"},{"line_number":99,"context_line":"                     \u0027creates the image in Glance. This can cause problems \u0027"},{"line_number":100,"context_line":"                     \u0027for image metadata that are in namespaces that glance \u0027"},{"line_number":101,"context_line":"                     \u0027reserves for itself, or when properties (such as an \u0027"}],"source_content_type":"text/x-python","patch_set":38,"id":"5c997927_89078e3c","line":98,"range":{"start_line":98,"start_character":22,"end_line":98,"end_character":23},"updated":"2023-01-30 06:39:34.000000000","message":"nit: an","commit_id":"9a4d1f279cf9376b95090ac7b7d0589ed7d8c7f1"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"009d3843227e8c6bb910a8ed32ca65b4be688bda","unresolved":false,"context_lines":[{"line_number":95,"context_line":"                     \u0027to Glance. When a volume is created from an image, \u0027"},{"line_number":96,"context_line":"                     \u0027Cinder stores the image properties as volume \u0027"},{"line_number":97,"context_line":"                     \u0027image_metadata, and if the volume is later uploaded as \u0027"},{"line_number":98,"context_line":"                     \u0027a image, Cinder will add these properties when it \u0027"},{"line_number":99,"context_line":"                     \u0027creates the image in Glance. This can cause problems \u0027"},{"line_number":100,"context_line":"                     \u0027for image metadata that are in namespaces that glance \u0027"},{"line_number":101,"context_line":"                     \u0027reserves for itself, or when properties (such as an \u0027"}],"source_content_type":"text/x-python","patch_set":38,"id":"b92272fc_96f0c63c","line":98,"range":{"start_line":98,"start_character":22,"end_line":98,"end_character":23},"in_reply_to":"5c997927_89078e3c","updated":"2023-01-30 11:45:53.000000000","message":"Done","commit_id":"9a4d1f279cf9376b95090ac7b7d0589ed7d8c7f1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d39de6c8cc37ce02c1e63908310fe4719ce8e988","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                     \u0027reserves for itself, or when properties (such as an \u0027"},{"line_number":102,"context_line":"                     \u0027image signature) cannot apply to the new image, or when \u0027"},{"line_number":103,"context_line":"                     \u0027an operator has configured glance property protections \u0027"},{"line_number":104,"context_line":"                     \u0027to make some image properties read-only. Cinder will \u0027"},{"line_number":105,"context_line":"                     \u0027*always* filter out image metadata in the namespaces \u0027"},{"line_number":106,"context_line":"                     \u0027`os_glance` and `img_signature`; this configuration \u0027"},{"line_number":107,"context_line":"                     \u0027option allows operators to specify *additional* \u0027"},{"line_number":108,"context_line":"                     \u0027namespaces to be excluded.\u0027,"},{"line_number":109,"context_line":"                default\u003d[]),"}],"source_content_type":"text/x-python","patch_set":38,"id":"ea6f48b3_ce63711b","line":106,"range":{"start_line":104,"start_character":63,"end_line":106,"end_character":53},"updated":"2023-01-30 06:39:34.000000000","message":"+1, this will avoid operators configuring os_glance or img_signature as a value for this config option.","commit_id":"9a4d1f279cf9376b95090ac7b7d0589ed7d8c7f1"}],"cinder/tests/unit/test_image_utils.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"307aac69f919ec311b0fc7924f0c6ccc4b89e403","unresolved":true,"context_lines":[{"line_number":2118,"context_line":"                          \u0027aes\u0027,"},{"line_number":2119,"context_line":"                          256)"},{"line_number":2120,"context_line":""},{"line_number":2121,"context_line":""},{"line_number":2122,"context_line":"class TestFilterReservedNamespaces(test.TestCase):"},{"line_number":2123,"context_line":""},{"line_number":2124,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"7bfb656d_382ad90c","line":2121,"updated":"2022-11-02 14:43:59.000000000","message":"I don\u0027t see the config option \"reserved_image_namespaces\" tested anywhere.\nWe will require a test that will override it and check if other properties are excluded as well based on the values set in the config option.","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"347fad509963aa776c906000383ccd696bdece39","unresolved":false,"context_lines":[{"line_number":2118,"context_line":"                          \u0027aes\u0027,"},{"line_number":2119,"context_line":"                          256)"},{"line_number":2120,"context_line":""},{"line_number":2121,"context_line":""},{"line_number":2122,"context_line":"class TestFilterReservedNamespaces(test.TestCase):"},{"line_number":2123,"context_line":""},{"line_number":2124,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"ea3a4242_1bf57722","line":2121,"in_reply_to":"7bfb656d_382ad90c","updated":"2022-11-08 16:18:09.000000000","message":"Done. Indeed there was a bug there in the algorithm! Nice catch!","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"eb57210640f582639c299f97ae05cc4ecfcf2660","unresolved":true,"context_lines":[{"line_number":2125,"context_line":""},{"line_number":2126,"context_line":"    def setUp(self):"},{"line_number":2127,"context_line":"        super(TestFilterReservedNamespaces, self).setUp()"},{"line_number":2128,"context_line":"        image_utils.LOG \u003d mock.MagicMock(side_effect\u003dimage_utils.LOG)"},{"line_number":2129,"context_line":""},{"line_number":2130,"context_line":"    def test_filter_out_reserved_namespaces_metadata_with_empty_metadata(self):"},{"line_number":2131,"context_line":"        metadata_for_test \u003d None"}],"source_content_type":"text/x-python","patch_set":18,"id":"5ff3689d_2b3fb333","line":2128,"range":{"start_line":2128,"start_character":8,"end_line":2128,"end_character":69},"updated":"2022-11-10 08:15:19.000000000","message":"this should be done with mock_object that automatically adds a cleanup for this mocking.\nWe already addressed a lot of issues related to this so would be better to not do mocking in this way.\nAn example for mock_object is here: https://github.com/openstack/cinder/blob/dcec2f6f01ffc63dc058f641370e9f5a0bad07e4/cinder/tests/unit/volume/drivers/test_nfs.py#L487-L488","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2993b8aafaeef5c1443701995168de839bb3f8bb","unresolved":false,"context_lines":[{"line_number":2125,"context_line":""},{"line_number":2126,"context_line":"    def setUp(self):"},{"line_number":2127,"context_line":"        super(TestFilterReservedNamespaces, self).setUp()"},{"line_number":2128,"context_line":"        image_utils.LOG \u003d mock.MagicMock(side_effect\u003dimage_utils.LOG)"},{"line_number":2129,"context_line":""},{"line_number":2130,"context_line":"    def test_filter_out_reserved_namespaces_metadata_with_empty_metadata(self):"},{"line_number":2131,"context_line":"        metadata_for_test \u003d None"}],"source_content_type":"text/x-python","patch_set":18,"id":"ab2b1db7_c8020239","line":2128,"range":{"start_line":2128,"start_character":8,"end_line":2128,"end_character":69},"in_reply_to":"5ff3689d_2b3fb333","updated":"2022-11-10 20:01:27.000000000","message":"Done","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ec62702b0e248cafc738c3a5fd93d57c83a8a65b","unresolved":true,"context_lines":[{"line_number":2152,"context_line":""},{"line_number":2153,"context_line":"    def test_filter_out_reserved_namespaces_metadata(self):"},{"line_number":2154,"context_line":"        metadata_for_test \u003d {\"some_key\": 13, \"other_key\": \"test\","},{"line_number":2155,"context_line":"                             \"os_glance_key\": \"this should be removed\"}"},{"line_number":2156,"context_line":"        method_return \u003d image_utils.filter_out_reserved_namespaces_metadata("},{"line_number":2157,"context_line":"            metadata_for_test)"},{"line_number":2158,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"9a42a817_36cee6a2","line":2155,"updated":"2022-11-09 23:44:51.000000000","message":"nit: when creating test data for a function that probably does some looping, you want to make sure the code doesn\u0027t exit early when it finds the first item to remove.  So I suggest scattering two \u0027os_glance*\u0027 keys among the test data.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2993b8aafaeef5c1443701995168de839bb3f8bb","unresolved":false,"context_lines":[{"line_number":2152,"context_line":""},{"line_number":2153,"context_line":"    def test_filter_out_reserved_namespaces_metadata(self):"},{"line_number":2154,"context_line":"        metadata_for_test \u003d {\"some_key\": 13, \"other_key\": \"test\","},{"line_number":2155,"context_line":"                             \"os_glance_key\": \"this should be removed\"}"},{"line_number":2156,"context_line":"        method_return \u003d image_utils.filter_out_reserved_namespaces_metadata("},{"line_number":2157,"context_line":"            metadata_for_test)"},{"line_number":2158,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"cbb65440_bf355200","line":2155,"in_reply_to":"9a42a817_36cee6a2","updated":"2022-11-10 20:01:27.000000000","message":"Done","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ec62702b0e248cafc738c3a5fd93d57c83a8a65b","unresolved":true,"context_lines":[{"line_number":2167,"context_line":"                      image_utils.CONF.reserved_image_namespaces,"},{"line_number":2168,"context_line":"                      expected_result)"},{"line_number":2169,"context_line":"        ])"},{"line_number":2170,"context_line":""},{"line_number":2171,"context_line":"    def test_filter_out_reserved_namespaces_metadata_custom_config(self):"},{"line_number":2172,"context_line":"        metadata_for_test \u003d {\"some_key\": 13, \"other_key\": \"test\","},{"line_number":2173,"context_line":"                             \"custom_key\": \"this should be removed\","}],"source_content_type":"text/x-python","patch_set":18,"id":"d86aea82_9fa82899","line":2170,"updated":"2022-11-09 23:44:51.000000000","message":"I suggest adding another test that doesn\u0027t contain any os_glance* keys just to show that your code doesn\u0027t modify a dict that doesn\u0027t need anything removed.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2993b8aafaeef5c1443701995168de839bb3f8bb","unresolved":false,"context_lines":[{"line_number":2167,"context_line":"                      image_utils.CONF.reserved_image_namespaces,"},{"line_number":2168,"context_line":"                      expected_result)"},{"line_number":2169,"context_line":"        ])"},{"line_number":2170,"context_line":""},{"line_number":2171,"context_line":"    def test_filter_out_reserved_namespaces_metadata_custom_config(self):"},{"line_number":2172,"context_line":"        metadata_for_test \u003d {\"some_key\": 13, \"other_key\": \"test\","},{"line_number":2173,"context_line":"                             \"custom_key\": \"this should be removed\","}],"source_content_type":"text/x-python","patch_set":18,"id":"2f392816_d3b842bd","line":2170,"in_reply_to":"d86aea82_9fa82899","updated":"2022-11-10 20:01:27.000000000","message":"Done","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ec62702b0e248cafc738c3a5fd93d57c83a8a65b","unresolved":true,"context_lines":[{"line_number":2171,"context_line":"    def test_filter_out_reserved_namespaces_metadata_custom_config(self):"},{"line_number":2172,"context_line":"        metadata_for_test \u003d {\"some_key\": 13, \"other_key\": \"test\","},{"line_number":2173,"context_line":"                             \"custom_key\": \"this should be removed\","},{"line_number":2174,"context_line":"                             \"another_custom_key\": \"this should be removed\"}"},{"line_number":2175,"context_line":"        self.override_config("},{"line_number":2176,"context_line":"            \u0027reserved_image_namespaces\u0027, [\u0027custom_key\u0027, \u0027another_custom_key\u0027])"},{"line_number":2177,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"768039b2_d40c0576","line":2174,"updated":"2022-11-09 23:44:51.000000000","message":"You might want to include an os_glance_key in the test data (which won\u0027t be removed) to show that your code is using only the conf option and the \u0027os_glance\u0027 namespace isn\u0027t hard-coded anywhere.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"eb57210640f582639c299f97ae05cc4ecfcf2660","unresolved":true,"context_lines":[{"line_number":2171,"context_line":"    def test_filter_out_reserved_namespaces_metadata_custom_config(self):"},{"line_number":2172,"context_line":"        metadata_for_test \u003d {\"some_key\": 13, \"other_key\": \"test\","},{"line_number":2173,"context_line":"                             \"custom_key\": \"this should be removed\","},{"line_number":2174,"context_line":"                             \"another_custom_key\": \"this should be removed\"}"},{"line_number":2175,"context_line":"        self.override_config("},{"line_number":2176,"context_line":"            \u0027reserved_image_namespaces\u0027, [\u0027custom_key\u0027, \u0027another_custom_key\u0027])"},{"line_number":2177,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"b5dc28ea_fa48c2c8","line":2174,"in_reply_to":"768039b2_d40c0576","updated":"2022-11-10 08:15:19.000000000","message":"I think a ddt would be better for (metadata and \"reserved_image_namespaces\" config option) that would cover the current 2 test cases and also address Brian\u0027s concerns.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2729cc1dc3a8406951a11c41c481c055fae38707","unresolved":true,"context_lines":[{"line_number":2171,"context_line":"    def test_filter_out_reserved_namespaces_metadata_custom_config(self):"},{"line_number":2172,"context_line":"        metadata_for_test \u003d {\"some_key\": 13, \"other_key\": \"test\","},{"line_number":2173,"context_line":"                             \"custom_key\": \"this should be removed\","},{"line_number":2174,"context_line":"                             \"another_custom_key\": \"this should be removed\"}"},{"line_number":2175,"context_line":"        self.override_config("},{"line_number":2176,"context_line":"            \u0027reserved_image_namespaces\u0027, [\u0027custom_key\u0027, \u0027another_custom_key\u0027])"},{"line_number":2177,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"d1589e94_4e496759","line":2174,"in_reply_to":"b5dc28ea_fa48c2c8","updated":"2022-11-10 13:02:55.000000000","message":"I agree with Rajat, ddt is the way to go here.  There are some examples above in this file, or you can do something more elaborate (that may be easier for people to read) like this: https://opendev.org/openstack/cinder/src/commit/dcec2f6f01ffc63dc058f641370e9f5a0bad07e4/cinder/tests/unit/volume/flows/test_create_volume_flow.py#L1423-L1476","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2993b8aafaeef5c1443701995168de839bb3f8bb","unresolved":false,"context_lines":[{"line_number":2171,"context_line":"    def test_filter_out_reserved_namespaces_metadata_custom_config(self):"},{"line_number":2172,"context_line":"        metadata_for_test \u003d {\"some_key\": 13, \"other_key\": \"test\","},{"line_number":2173,"context_line":"                             \"custom_key\": \"this should be removed\","},{"line_number":2174,"context_line":"                             \"another_custom_key\": \"this should be removed\"}"},{"line_number":2175,"context_line":"        self.override_config("},{"line_number":2176,"context_line":"            \u0027reserved_image_namespaces\u0027, [\u0027custom_key\u0027, \u0027another_custom_key\u0027])"},{"line_number":2177,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"06dea5d4_21331f33","line":2174,"in_reply_to":"d1589e94_4e496759","updated":"2022-11-10 20:01:27.000000000","message":"Done","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b2f3426bfc7309e9bfe0ecce42721e4c8c664e23","unresolved":true,"context_lines":[{"line_number":2181,"context_line":"        all_keys_expected_result \u003d expected_result.keys()"},{"line_number":2182,"context_line":"        for key in list(all_keys_expected_result):"},{"line_number":2183,"context_line":"            for key_to_pop in keys_to_pop:"},{"line_number":2184,"context_line":"                if key_to_pop in key and expected_result.get(key):"},{"line_number":2185,"context_line":"                    expected_result.pop(key)"},{"line_number":2186,"context_line":""},{"line_number":2187,"context_line":"        self.assertEqual(expected_result, method_return)"}],"source_content_type":"text/x-python","patch_set":28,"id":"2cfd9db4_af7b6c92","line":2184,"range":{"start_line":2184,"start_character":16,"end_line":2184,"end_character":36},"updated":"2022-12-20 17:22:24.000000000","message":"This is a wrong check here and doesn\u0027t match with the actual code. I was able to fail this test by setting,\nmetadata \u003d {\"other_custom_key\": 123} and config\u003d[\u0027custom_key\u0027]\n\nThe original code with check startswith which will not filter out this key but this condition will filter this out since \u0027custom_key\u0027 exists in \u0027other_custom_key\u0027","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"675fbea02d69f1ff56f270ecff4156a6ea0b7150","unresolved":true,"context_lines":[{"line_number":2181,"context_line":"        all_keys_expected_result \u003d expected_result.keys()"},{"line_number":2182,"context_line":"        for key in list(all_keys_expected_result):"},{"line_number":2183,"context_line":"            for key_to_pop in keys_to_pop:"},{"line_number":2184,"context_line":"                if key_to_pop in key and expected_result.get(key):"},{"line_number":2185,"context_line":"                    expected_result.pop(key)"},{"line_number":2186,"context_line":""},{"line_number":2187,"context_line":"        self.assertEqual(expected_result, method_return)"}],"source_content_type":"text/x-python","patch_set":28,"id":"4f3c4941_25dfae7d","line":2184,"range":{"start_line":2184,"start_character":16,"end_line":2184,"end_character":36},"in_reply_to":"2cfd9db4_af7b6c92","updated":"2022-12-20 17:24:04.000000000","message":"Also i prefer keeping a expected_result dict hardcoded so it\u0027s easy to review and see which all properties we are comparing the results with rather than implementing the same logic in test cases.\nIt works here since the logic is simple but for large and complex methods, we should prefer the hardcoded expected results.","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"ae22d464c0f7428a48f181add1322202d1f0dfd2","unresolved":false,"context_lines":[{"line_number":2181,"context_line":"        all_keys_expected_result \u003d expected_result.keys()"},{"line_number":2182,"context_line":"        for key in list(all_keys_expected_result):"},{"line_number":2183,"context_line":"            for key_to_pop in keys_to_pop:"},{"line_number":2184,"context_line":"                if key_to_pop in key and expected_result.get(key):"},{"line_number":2185,"context_line":"                    expected_result.pop(key)"},{"line_number":2186,"context_line":""},{"line_number":2187,"context_line":"        self.assertEqual(expected_result, method_return)"}],"source_content_type":"text/x-python","patch_set":28,"id":"15482073_d48c4732","line":2184,"range":{"start_line":2184,"start_character":16,"end_line":2184,"end_character":36},"in_reply_to":"4f3c4941_25dfae7d","updated":"2022-12-21 11:26:43.000000000","message":"Done","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"9c90ae4e007b9abd3d47f5bb32f5937b048b6fbb","unresolved":false,"context_lines":[{"line_number":2173,"context_line":"        if config:"},{"line_number":2174,"context_line":"            self.override_config(\u0027reserved_image_namespaces\u0027, config)"},{"line_number":2175,"context_line":""},{"line_number":2176,"context_line":"        expected_result \u003d {\"some_key\": 13, \"other_key\": \"test\"}"},{"line_number":2177,"context_line":""},{"line_number":2178,"context_line":"        method_return \u003d image_utils.filter_out_reserved_namespaces_metadata("},{"line_number":2179,"context_line":"            metadata_for_test)"}],"source_content_type":"text/x-python","patch_set":29,"id":"bd1a8ebc_bf0189f5","line":2176,"range":{"start_line":2176,"start_character":26,"end_line":2176,"end_character":63},"updated":"2022-12-21 18:00:37.000000000","message":"this works because in all cases we end up with same keys","commit_id":"9726c315e29cbdde703b56cf4615e40965a4f0af"}],"cinder/utils.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6220a1c9bd2b5e51910d4bbf97192da92d3cf002","unresolved":true,"context_lines":[{"line_number":950,"context_line":"            return func(self, *args, **kwargs)"},{"line_number":951,"context_line":"    return wrapper"},{"line_number":952,"context_line":""},{"line_number":953,"context_line":""},{"line_number":954,"context_line":"RESERVED_NAME_SPACES \u003d [\"os_glance\"]"},{"line_number":955,"context_line":""},{"line_number":956,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3525dddd_4e9bef53","line":953,"updated":"2021-10-07 12:00:08.000000000","message":"Question: why in \u0027utils\u0027 instead of \u0027image_utils\u0027 ?","commit_id":"eceaa95aa2f2b46d16df40f2af97d4b6d434842a"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"9505ab8453c1e28c5fd4d270d7fba03622f64072","unresolved":true,"context_lines":[{"line_number":950,"context_line":"            return func(self, *args, **kwargs)"},{"line_number":951,"context_line":"    return wrapper"},{"line_number":952,"context_line":""},{"line_number":953,"context_line":""},{"line_number":954,"context_line":"RESERVED_NAME_SPACES \u003d [\"os_glance\"]"},{"line_number":955,"context_line":""},{"line_number":956,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"50517f46_9b3afb6b","line":953,"in_reply_to":"3525dddd_4e9bef53","updated":"2021-10-07 12:19:53.000000000","message":"I did not see the image_utils. I can move the code there if you prefer.","commit_id":"eceaa95aa2f2b46d16df40f2af97d4b6d434842a"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"102e085b0d0865c383af4ddd35f220880f9a713a","unresolved":false,"context_lines":[{"line_number":950,"context_line":"            return func(self, *args, **kwargs)"},{"line_number":951,"context_line":"    return wrapper"},{"line_number":952,"context_line":""},{"line_number":953,"context_line":""},{"line_number":954,"context_line":"RESERVED_NAME_SPACES \u003d [\"os_glance\"]"},{"line_number":955,"context_line":""},{"line_number":956,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"74767972_1485f640","line":953,"in_reply_to":"50517f46_9b3afb6b","updated":"2022-06-29 11:51:14.000000000","message":"Done","commit_id":"eceaa95aa2f2b46d16df40f2af97d4b6d434842a"}],"cinder/volume/api.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"5ad32039df3aef4ce8e3f6489dccac95a2737bb7","unresolved":true,"context_lines":[{"line_number":1437,"context_line":"                        \"for volume [%s].\", all_metadata, volume[\u0027id\u0027])"},{"line_number":1438,"context_line":"            return all_metadata[0]"},{"line_number":1439,"context_line":"        else:"},{"line_number":1440,"context_line":"            return []"},{"line_number":1441,"context_line":""},{"line_number":1442,"context_line":"    def get_list_volumes_image_metadata("},{"line_number":1443,"context_line":"            self,"}],"source_content_type":"text/x-python","patch_set":12,"id":"0b38b76f_e500528e","line":1440,"range":{"start_line":1440,"start_character":0,"end_line":1440,"end_character":1},"updated":"2022-06-29 15:02:59.000000000","message":"This introduces a failure in the cinder-mypy job -- please look at that.","commit_id":"77ae30bd8bbbdf3deb3fa24e6b4388510891e1b2"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"e980bd444256539d6c414c01cb56bf104d3757b4","unresolved":false,"context_lines":[{"line_number":1437,"context_line":"                        \"for volume [%s].\", all_metadata, volume[\u0027id\u0027])"},{"line_number":1438,"context_line":"            return all_metadata[0]"},{"line_number":1439,"context_line":"        else:"},{"line_number":1440,"context_line":"            return []"},{"line_number":1441,"context_line":""},{"line_number":1442,"context_line":"    def get_list_volumes_image_metadata("},{"line_number":1443,"context_line":"            self,"}],"source_content_type":"text/x-python","patch_set":12,"id":"a21f2049_2fe0e045","line":1440,"range":{"start_line":1440,"start_character":0,"end_line":1440,"end_character":1},"in_reply_to":"0b38b76f_e500528e","updated":"2022-06-30 14:40:44.000000000","message":"Done","commit_id":"77ae30bd8bbbdf3deb3fa24e6b4388510891e1b2"},{"author":{"_account_id":35391,"name":"Andrey Sakhno","email":"sakhno.a@selectel.ru"},"change_message_id":"5a09b63dcc5abbea4e7253b65a43c624d0f02123","unresolved":true,"context_lines":[{"line_number":1437,"context_line":"                        \"for volume [%s].\", all_metadata, volume[\u0027id\u0027])"},{"line_number":1438,"context_line":"            return all_metadata[0]"},{"line_number":1439,"context_line":"        else:"},{"line_number":1440,"context_line":"            return collections.defaultdict()"},{"line_number":1441,"context_line":""},{"line_number":1442,"context_line":"    def get_list_volumes_image_metadata("},{"line_number":1443,"context_line":"            self,"}],"source_content_type":"text/x-python","patch_set":15,"id":"5e35bc7c_cad9c61f","line":1440,"updated":"2022-10-12 08:12:31.000000000","message":"here should `return {}` because all_metadata type is `List[dict]`, also type hint of return value type should fix","commit_id":"e47ee3f8d1f5c5b1cf3f68af7241baf1854e3853"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"00a05685edd259f3e80ff127b51a154578e9bea9","unresolved":false,"context_lines":[{"line_number":1437,"context_line":"                        \"for volume [%s].\", all_metadata, volume[\u0027id\u0027])"},{"line_number":1438,"context_line":"            return all_metadata[0]"},{"line_number":1439,"context_line":"        else:"},{"line_number":1440,"context_line":"            return collections.defaultdict()"},{"line_number":1441,"context_line":""},{"line_number":1442,"context_line":"    def get_list_volumes_image_metadata("},{"line_number":1443,"context_line":"            self,"}],"source_content_type":"text/x-python","patch_set":15,"id":"c38500d4_aff9f738","line":1440,"in_reply_to":"5e35bc7c_cad9c61f","updated":"2022-10-12 11:46:14.000000000","message":"done.","commit_id":"e47ee3f8d1f5c5b1cf3f68af7241baf1854e3853"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"0f9e7fd3dabf5ee876cc9b37fb404d0d069bb898","unresolved":false,"context_lines":[{"line_number":1437,"context_line":"                        \"for volume [%s].\", all_metadata, volume[\u0027id\u0027])"},{"line_number":1438,"context_line":"            return all_metadata[0]"},{"line_number":1439,"context_line":"        else:"},{"line_number":1440,"context_line":"            return collections.defaultdict()"},{"line_number":1441,"context_line":""},{"line_number":1442,"context_line":"    def get_list_volumes_image_metadata("},{"line_number":1443,"context_line":"            self,"}],"source_content_type":"text/x-python","patch_set":15,"id":"35715419_88b28ef8","line":1440,"in_reply_to":"c38500d4_aff9f738","updated":"2022-10-12 13:53:03.000000000","message":"The problem is that doing what you suggested breaks other tests. Do you have something else in your mind?","commit_id":"e47ee3f8d1f5c5b1cf3f68af7241baf1854e3853"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"307aac69f919ec311b0fc7924f0c6ccc4b89e403","unresolved":true,"context_lines":[{"line_number":1399,"context_line":"                 resource\u003dsnapshot)"},{"line_number":1400,"context_line":"        return snapshot.metadata"},{"line_number":1401,"context_line":""},{"line_number":1402,"context_line":"    @staticmethod"},{"line_number":1403,"context_line":"    def generate_volumes_response(db_data) -\u003e collections.defaultdict:"},{"line_number":1404,"context_line":"        results: collections.defaultdict \u003d collections.defaultdict(dict)"},{"line_number":1405,"context_line":"        for meta_entry in db_data:"},{"line_number":1406,"context_line":"            meta \u003d {meta_entry[\u0027key\u0027]: meta_entry[\u0027value\u0027]}"},{"line_number":1407,"context_line":"            meta \u003d image_utils.filter_out_reserved_namespaces_metadata(meta)"},{"line_number":1408,"context_line":"            if meta:"},{"line_number":1409,"context_line":"                results[meta_entry[\u0027volume_id\u0027]].update(meta)"},{"line_number":1410,"context_line":""},{"line_number":1411,"context_line":"        return results"},{"line_number":1412,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"eda3bcfb_0f932f70","line":1409,"range":{"start_line":1402,"start_character":0,"end_line":1409,"end_character":61},"updated":"2022-11-02 14:43:59.000000000","message":"is this done for existing volumes having the os_glance properties since we don\u0027t include those properties in new volumes?\nwouldn\u0027t it be better to remove it for existing volumes as well rather than filtering it out every time we do a get request?","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"eb57210640f582639c299f97ae05cc4ecfcf2660","unresolved":false,"context_lines":[{"line_number":1399,"context_line":"                 resource\u003dsnapshot)"},{"line_number":1400,"context_line":"        return snapshot.metadata"},{"line_number":1401,"context_line":""},{"line_number":1402,"context_line":"    @staticmethod"},{"line_number":1403,"context_line":"    def generate_volumes_response(db_data) -\u003e collections.defaultdict:"},{"line_number":1404,"context_line":"        results: collections.defaultdict \u003d collections.defaultdict(dict)"},{"line_number":1405,"context_line":"        for meta_entry in db_data:"},{"line_number":1406,"context_line":"            meta \u003d {meta_entry[\u0027key\u0027]: meta_entry[\u0027value\u0027]}"},{"line_number":1407,"context_line":"            meta \u003d image_utils.filter_out_reserved_namespaces_metadata(meta)"},{"line_number":1408,"context_line":"            if meta:"},{"line_number":1409,"context_line":"                results[meta_entry[\u0027volume_id\u0027]].update(meta)"},{"line_number":1410,"context_line":""},{"line_number":1411,"context_line":"        return results"},{"line_number":1412,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3dec0619_650006b6","line":1409,"range":{"start_line":1402,"start_character":0,"end_line":1409,"end_character":61},"in_reply_to":"7017560c_ab2974d8","updated":"2022-11-10 08:15:19.000000000","message":"Makes sense, I think a DB migration in the next cycle based on the config option \u0027reserved_image_namespaces\u0027 would be good. We can remove the values set as \u0027reserved_image_namespaces\u0027 in the existing volumes in that migration.","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"64f14dd85fffb49f7f46d726e272a6585420a730","unresolved":false,"context_lines":[{"line_number":1399,"context_line":"                 resource\u003dsnapshot)"},{"line_number":1400,"context_line":"        return snapshot.metadata"},{"line_number":1401,"context_line":""},{"line_number":1402,"context_line":"    @staticmethod"},{"line_number":1403,"context_line":"    def generate_volumes_response(db_data) -\u003e collections.defaultdict:"},{"line_number":1404,"context_line":"        results: collections.defaultdict \u003d collections.defaultdict(dict)"},{"line_number":1405,"context_line":"        for meta_entry in db_data:"},{"line_number":1406,"context_line":"            meta \u003d {meta_entry[\u0027key\u0027]: meta_entry[\u0027value\u0027]}"},{"line_number":1407,"context_line":"            meta \u003d image_utils.filter_out_reserved_namespaces_metadata(meta)"},{"line_number":1408,"context_line":"            if meta:"},{"line_number":1409,"context_line":"                results[meta_entry[\u0027volume_id\u0027]].update(meta)"},{"line_number":1410,"context_line":""},{"line_number":1411,"context_line":"        return results"},{"line_number":1412,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"7017560c_ab2974d8","line":1409,"range":{"start_line":1402,"start_character":0,"end_line":1409,"end_character":61},"in_reply_to":"a7eaea28_65630c69","updated":"2022-11-08 16:36:46.000000000","message":"Rodrigo, thanks for taking the time to address these questions!","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"347fad509963aa776c906000383ccd696bdece39","unresolved":false,"context_lines":[{"line_number":1399,"context_line":"                 resource\u003dsnapshot)"},{"line_number":1400,"context_line":"        return snapshot.metadata"},{"line_number":1401,"context_line":""},{"line_number":1402,"context_line":"    @staticmethod"},{"line_number":1403,"context_line":"    def generate_volumes_response(db_data) -\u003e collections.defaultdict:"},{"line_number":1404,"context_line":"        results: collections.defaultdict \u003d collections.defaultdict(dict)"},{"line_number":1405,"context_line":"        for meta_entry in db_data:"},{"line_number":1406,"context_line":"            meta \u003d {meta_entry[\u0027key\u0027]: meta_entry[\u0027value\u0027]}"},{"line_number":1407,"context_line":"            meta \u003d image_utils.filter_out_reserved_namespaces_metadata(meta)"},{"line_number":1408,"context_line":"            if meta:"},{"line_number":1409,"context_line":"                results[meta_entry[\u0027volume_id\u0027]].update(meta)"},{"line_number":1410,"context_line":""},{"line_number":1411,"context_line":"        return results"},{"line_number":1412,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"a7eaea28_65630c69","line":1409,"range":{"start_line":1402,"start_character":0,"end_line":1409,"end_character":61},"in_reply_to":"eda3bcfb_0f932f70","updated":"2022-11-08 16:18:09.000000000","message":"I\u0027m assuming what you\u0027re asking has no relation with the code above, as that is just a refactored code without change in behavior.\n\nFor removing it from existing volumes I believe a DB migration entry would be necessary. Indeed that could be done, as this patch will not remove existing, but preventing new metadata to be saved from/to glance images, while old metadata will remain without causing issues. However, IMO, we should not delete existing metadata without warning the operator of it. If the operator does not agree with the new filter, they can just configure it to stop filtering.","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"307aac69f919ec311b0fc7924f0c6ccc4b89e403","unresolved":true,"context_lines":[{"line_number":1430,"context_line":"        LOG.info(\"Get volume image-metadata %s completed successfully for \""},{"line_number":1431,"context_line":"                 \"volume [%s].\", all_metadata, volume[\u0027id\u0027])"},{"line_number":1432,"context_line":""},{"line_number":1433,"context_line":"        if len(all_metadata) \u003d\u003d 1:"},{"line_number":1434,"context_line":"            return all_metadata[0]"},{"line_number":1435,"context_line":"        elif len(all_metadata) \u003e 1:"},{"line_number":1436,"context_line":"            LOG.warning(\"There is something unexpected with the metadata [%] \""},{"line_number":1437,"context_line":"                        \"for volume [%s].\", all_metadata, volume[\u0027id\u0027])"},{"line_number":1438,"context_line":"            return all_metadata[0]"},{"line_number":1439,"context_line":"        else:"},{"line_number":1440,"context_line":"            return collections.defaultdict()"}],"source_content_type":"text/x-python","patch_set":17,"id":"06aee8e7_7675e7ec","line":1437,"range":{"start_line":1433,"start_character":8,"end_line":1437,"end_character":71},"updated":"2022-11-02 14:43:59.000000000","message":"I don\u0027t understand these conditions, what are we exactly trying to do here?","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"347fad509963aa776c906000383ccd696bdece39","unresolved":false,"context_lines":[{"line_number":1430,"context_line":"        LOG.info(\"Get volume image-metadata %s completed successfully for \""},{"line_number":1431,"context_line":"                 \"volume [%s].\", all_metadata, volume[\u0027id\u0027])"},{"line_number":1432,"context_line":""},{"line_number":1433,"context_line":"        if len(all_metadata) \u003d\u003d 1:"},{"line_number":1434,"context_line":"            return all_metadata[0]"},{"line_number":1435,"context_line":"        elif len(all_metadata) \u003e 1:"},{"line_number":1436,"context_line":"            LOG.warning(\"There is something unexpected with the metadata [%] \""},{"line_number":1437,"context_line":"                        \"for volume [%s].\", all_metadata, volume[\u0027id\u0027])"},{"line_number":1438,"context_line":"            return all_metadata[0]"},{"line_number":1439,"context_line":"        else:"},{"line_number":1440,"context_line":"            return collections.defaultdict()"}],"source_content_type":"text/x-python","patch_set":17,"id":"5582cd26_e595352c","line":1437,"range":{"start_line":1433,"start_character":8,"end_line":1437,"end_character":71},"in_reply_to":"06aee8e7_7675e7ec","updated":"2022-11-08 16:18:09.000000000","message":"these conditions are formatting the result value. The old methods return a key-value pair of metadata, but here the author has refactored several of the old methods into a single method in \"generate_volumes_response\", which has a return value indexed by a volume_id (a dict of dicts). That matches the return value of \"get_volumes_image_metadata\" and \"get_list_volumes_image_metadata\" methods, which are indexed by volume_id, but for this method it has to return it not indexed by volume_id. The conditions are there to produce a warning in case there is more than one entry, given the key-value pair should be related to only a volume, there shouldn\u0027t be more than one.","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"64f14dd85fffb49f7f46d726e272a6585420a730","unresolved":false,"context_lines":[{"line_number":1430,"context_line":"        LOG.info(\"Get volume image-metadata %s completed successfully for \""},{"line_number":1431,"context_line":"                 \"volume [%s].\", all_metadata, volume[\u0027id\u0027])"},{"line_number":1432,"context_line":""},{"line_number":1433,"context_line":"        if len(all_metadata) \u003d\u003d 1:"},{"line_number":1434,"context_line":"            return all_metadata[0]"},{"line_number":1435,"context_line":"        elif len(all_metadata) \u003e 1:"},{"line_number":1436,"context_line":"            LOG.warning(\"There is something unexpected with the metadata [%] \""},{"line_number":1437,"context_line":"                        \"for volume [%s].\", all_metadata, volume[\u0027id\u0027])"},{"line_number":1438,"context_line":"            return all_metadata[0]"},{"line_number":1439,"context_line":"        else:"},{"line_number":1440,"context_line":"            return collections.defaultdict()"}],"source_content_type":"text/x-python","patch_set":17,"id":"8eae3833_b651e55d","line":1437,"range":{"start_line":1433,"start_character":8,"end_line":1437,"end_character":71},"in_reply_to":"5582cd26_e595352c","updated":"2022-11-08 16:36:46.000000000","message":"Exactly that. I just tried to remove some replicated code.","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"}],"cinder/volume/flows/manager/create_volume.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"307aac69f919ec311b0fc7924f0c6ccc4b89e403","unresolved":true,"context_lines":[{"line_number":320,"context_line":"                                                               image_href)"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"            image_meta \u003d image_service.show(context, image_id)"},{"line_number":323,"context_line":"            LOG.debug(\"Image [%s] metadata [%s] loaded from [%s].\","},{"line_number":324,"context_line":"                      image_id, image_meta, image_service)"},{"line_number":325,"context_line":"            image_meta \u003d image_utils.filter_out_reserved_namespaces_metadata("},{"line_number":326,"context_line":"                image_meta)"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"71a81e4e_62ac6eb8","line":324,"range":{"start_line":323,"start_character":12,"end_line":324,"end_character":58},"updated":"2022-11-02 14:43:59.000000000","message":"This is kind of misleading since we are removing the reserved attributes in the next line. If someone compares this debug log with the actual image_metadata in the volume, it will be a mismatch. It would be better to do the logging after we filter out the reserved properties.","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"eb57210640f582639c299f97ae05cc4ecfcf2660","unresolved":false,"context_lines":[{"line_number":320,"context_line":"                                                               image_href)"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"            image_meta \u003d image_service.show(context, image_id)"},{"line_number":323,"context_line":"            LOG.debug(\"Image [%s] metadata [%s] loaded from [%s].\","},{"line_number":324,"context_line":"                      image_id, image_meta, image_service)"},{"line_number":325,"context_line":"            image_meta \u003d image_utils.filter_out_reserved_namespaces_metadata("},{"line_number":326,"context_line":"                image_meta)"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"71e833ad_c5db4883","line":324,"range":{"start_line":323,"start_character":12,"end_line":324,"end_character":58},"in_reply_to":"6058b3c4_dd91302e","updated":"2022-11-10 08:15:19.000000000","message":"Maybe we can add a diff variable in the log that is difference of old image_meta and new image_meta (after filtering)","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"347fad509963aa776c906000383ccd696bdece39","unresolved":false,"context_lines":[{"line_number":320,"context_line":"                                                               image_href)"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"            image_meta \u003d image_service.show(context, image_id)"},{"line_number":323,"context_line":"            LOG.debug(\"Image [%s] metadata [%s] loaded from [%s].\","},{"line_number":324,"context_line":"                      image_id, image_meta, image_service)"},{"line_number":325,"context_line":"            image_meta \u003d image_utils.filter_out_reserved_namespaces_metadata("},{"line_number":326,"context_line":"                image_meta)"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"ca5e11b4_19fee9ad","line":324,"range":{"start_line":323,"start_character":12,"end_line":324,"end_character":58},"in_reply_to":"71a81e4e_62ac6eb8","updated":"2022-11-08 16:18:09.000000000","message":"Done","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"64f14dd85fffb49f7f46d726e272a6585420a730","unresolved":false,"context_lines":[{"line_number":320,"context_line":"                                                               image_href)"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"            image_meta \u003d image_service.show(context, image_id)"},{"line_number":323,"context_line":"            LOG.debug(\"Image [%s] metadata [%s] loaded from [%s].\","},{"line_number":324,"context_line":"                      image_id, image_meta, image_service)"},{"line_number":325,"context_line":"            image_meta \u003d image_utils.filter_out_reserved_namespaces_metadata("},{"line_number":326,"context_line":"                image_meta)"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"6058b3c4_dd91302e","line":324,"range":{"start_line":323,"start_character":12,"end_line":324,"end_character":58},"in_reply_to":"ca5e11b4_19fee9ad","updated":"2022-11-08 16:36:46.000000000","message":"I disagree with this removal. However, I really like logs to help operators. This line, with the others that would explicitly tell when the keys are being removed would make sense, and show clearly the processing for operators, if they needed to troubleshoot the system.","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"}],"releasenotes/notes/fix-reserved-image-properties-9519ddc080e7ed1a.yaml":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"307aac69f919ec311b0fc7924f0c6ccc4b89e403","unresolved":true,"context_lines":[{"line_number":6,"context_line":"    Cinder is currently not able to upload a volume"},{"line_number":7,"context_line":"    that is based on an image back to glance. This happens"},{"line_number":8,"context_line":"    if glance multistore is enabled. When enabling multistore,"},{"line_number":9,"context_line":"    the properties \"os_glance_failed_import\" and "},{"line_number":10,"context_line":"    \"os_glance_importing_to_stores\" will be stored in Cinder."},{"line_number":11,"context_line":"    These properties will cause problems when later, Cinder"},{"line_number":12,"context_line":"    tries to perform some actions with Glance."}],"source_content_type":"text/x-yaml","patch_set":17,"id":"80036c8a_c45207da","line":9,"range":{"start_line":9,"start_character":48,"end_line":9,"end_character":49},"updated":"2022-11-02 14:43:59.000000000","message":"nit: extra whitespace","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"347fad509963aa776c906000383ccd696bdece39","unresolved":false,"context_lines":[{"line_number":6,"context_line":"    Cinder is currently not able to upload a volume"},{"line_number":7,"context_line":"    that is based on an image back to glance. This happens"},{"line_number":8,"context_line":"    if glance multistore is enabled. When enabling multistore,"},{"line_number":9,"context_line":"    the properties \"os_glance_failed_import\" and "},{"line_number":10,"context_line":"    \"os_glance_importing_to_stores\" will be stored in Cinder."},{"line_number":11,"context_line":"    These properties will cause problems when later, Cinder"},{"line_number":12,"context_line":"    tries to perform some actions with Glance."}],"source_content_type":"text/x-yaml","patch_set":17,"id":"cf41c4c1_bbac91f4","line":9,"range":{"start_line":9,"start_character":48,"end_line":9,"end_character":49},"in_reply_to":"80036c8a_c45207da","updated":"2022-11-08 16:18:09.000000000","message":"Done","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"307aac69f919ec311b0fc7924f0c6ccc4b89e403","unresolved":true,"context_lines":[{"line_number":7,"context_line":"    that is based on an image back to glance. This happens"},{"line_number":8,"context_line":"    if glance multistore is enabled. When enabling multistore,"},{"line_number":9,"context_line":"    the properties \"os_glance_failed_import\" and "},{"line_number":10,"context_line":"    \"os_glance_importing_to_stores\" will be stored in Cinder."},{"line_number":11,"context_line":"    These properties will cause problems when later, Cinder"},{"line_number":12,"context_line":"    tries to perform some actions with Glance."}],"source_content_type":"text/x-yaml","patch_set":17,"id":"624f4c2e_eb2b369b","line":10,"range":{"start_line":10,"start_character":51,"end_line":10,"end_character":60},"updated":"2022-11-02 14:43:59.000000000","message":"not sure if it sounds right, it\u0027s a part of cinder volume image metadata.","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"347fad509963aa776c906000383ccd696bdece39","unresolved":false,"context_lines":[{"line_number":7,"context_line":"    that is based on an image back to glance. This happens"},{"line_number":8,"context_line":"    if glance multistore is enabled. When enabling multistore,"},{"line_number":9,"context_line":"    the properties \"os_glance_failed_import\" and "},{"line_number":10,"context_line":"    \"os_glance_importing_to_stores\" will be stored in Cinder."},{"line_number":11,"context_line":"    These properties will cause problems when later, Cinder"},{"line_number":12,"context_line":"    tries to perform some actions with Glance."}],"source_content_type":"text/x-yaml","patch_set":17,"id":"2ae18927_44091abc","line":10,"range":{"start_line":10,"start_character":51,"end_line":10,"end_character":60},"in_reply_to":"624f4c2e_eb2b369b","updated":"2022-11-08 16:18:09.000000000","message":"Done","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"307aac69f919ec311b0fc7924f0c6ccc4b89e403","unresolved":true,"context_lines":[{"line_number":9,"context_line":"    the properties \"os_glance_failed_import\" and "},{"line_number":10,"context_line":"    \"os_glance_importing_to_stores\" will be stored in Cinder."},{"line_number":11,"context_line":"    These properties will cause problems when later, Cinder"},{"line_number":12,"context_line":"    tries to perform some actions with Glance."}],"source_content_type":"text/x-yaml","patch_set":17,"id":"ef76aef8_9df3bbf7","line":12,"range":{"start_line":12,"start_character":39,"end_line":12,"end_character":45},"updated":"2022-11-02 14:43:59.000000000","message":"we can mention an example as well, like uploading volume to image.","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"347fad509963aa776c906000383ccd696bdece39","unresolved":false,"context_lines":[{"line_number":9,"context_line":"    the properties \"os_glance_failed_import\" and "},{"line_number":10,"context_line":"    \"os_glance_importing_to_stores\" will be stored in Cinder."},{"line_number":11,"context_line":"    These properties will cause problems when later, Cinder"},{"line_number":12,"context_line":"    tries to perform some actions with Glance."}],"source_content_type":"text/x-yaml","patch_set":17,"id":"e5f9e8ca_2d1f2dbb","line":12,"range":{"start_line":12,"start_character":39,"end_line":12,"end_character":45},"in_reply_to":"ef76aef8_9df3bbf7","updated":"2022-11-08 16:18:09.000000000","message":"Done","commit_id":"3a0cb6345c3e7dd2eef5ba26021ff909df5b1c57"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"eb57210640f582639c299f97ae05cc4ecfcf2660","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Filter reserved image properties used by Glance."},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"    Cinder is currently not able to upload a volume"},{"line_number":7,"context_line":"    that is based on an image back to glance. This happens"},{"line_number":8,"context_line":"    if glance multistore is enabled. When enabling multistore,"},{"line_number":9,"context_line":"    the properties \"os_glance_failed_import\" and"},{"line_number":10,"context_line":"    \"os_glance_importing_to_stores\" will be stored as metadata"},{"line_number":11,"context_line":"    in the cinder volume created from the glance image."},{"line_number":12,"context_line":"    These metadata will cause problems when later, it is"},{"line_number":13,"context_line":"    attempted to perform some actions with Glance, such as"},{"line_number":14,"context_line":"    uploading the volume."}],"source_content_type":"text/x-yaml","patch_set":18,"id":"332ca572_7d63b92b","line":14,"range":{"start_line":6,"start_character":4,"end_line":14,"end_character":25},"updated":"2022-11-10 08:15:19.000000000","message":"sorry for not mentioning this earlier but this has no mention of the config option we are adding, reserved_image_namespaces, to fix the issue.\n\nAlso it would be better to describe what is fixed and how it is done rather than explaining the problem statement again. Something like,\n\n-|\n  Filter reserved image properties used by Glance.\n\n  We introduced a new config parameter, ``reserved_image_namespaces``, that allows \n  operators to set the image properties to filter out from volume image metadata \n  when performing operations like creating a bootable volume. These properties, \n  if not filtered out, causes failures in further volume operations like \n  uploading the volume to image.","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"38216a023f1a4acd544376673935549f80b723db","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Filter reserved image properties used by Glance."},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"    Cinder is currently not able to upload a volume"},{"line_number":7,"context_line":"    that is based on an image back to glance. This happens"},{"line_number":8,"context_line":"    if glance multistore is enabled. When enabling multistore,"},{"line_number":9,"context_line":"    the properties \"os_glance_failed_import\" and"},{"line_number":10,"context_line":"    \"os_glance_importing_to_stores\" will be stored as metadata"},{"line_number":11,"context_line":"    in the cinder volume created from the glance image."},{"line_number":12,"context_line":"    These metadata will cause problems when later, it is"},{"line_number":13,"context_line":"    attempted to perform some actions with Glance, such as"},{"line_number":14,"context_line":"    uploading the volume."}],"source_content_type":"text/x-yaml","patch_set":18,"id":"ca43d7e6_117decb5","line":14,"range":{"start_line":6,"start_character":4,"end_line":14,"end_character":25},"in_reply_to":"332ca572_7d63b92b","updated":"2022-11-10 14:08:18.000000000","message":"Just to amplify Rajat\u0027s comment: the cinder release note guidelines are here:\nhttps://docs.openstack.org/cinder/latest/contributor/releasenotes.html\n\nA few things to keep in mind:\n\n- in the \u0027fixes\u0027 section, please use the \"Bug #1945500: Fixed whatever\" format.  Since there will be a link to the bug, you don\u0027t need to mention the particular glance properties, but can instead say something like \"Glance reserves image properties in the namespace \u0027os_glance\u0027 for its own use and will not allow images to be created with these properties.  This fix introduces a configuration option, xxx, that enables operators to specify a list of image property namespaces that will not be included when Cinder is requested to upload a volume as an image.  The default value is ``[\u0027os_glance\u0027]``.\n\n- the config opt should be mentioned in the \u0027fixes\u0027 section, but also again in the \u0027upgrade\u0027 section.  You can repeat most of the info, just reword it slightly to put the emphasis on the config opt.\n\n- i think you need to say \"filter by namespace\", because nova has 2 different options, one that allows a namespace filter, and one that allows a list of particular image properties, so we want operators to be clear about what this fix does.\n\n- you say that image properties are filtered, but you don\u0027t tell people what to expect, that is, is it being done on download, so that these properties are never stored in the volume_image_metadata, or is it being done on upload (the properties are stored in cinder, but they won\u0027t be created in glance if you do an upload-volume-as-image action).","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2993b8aafaeef5c1443701995168de839bb3f8bb","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Filter reserved image properties used by Glance."},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"    Cinder is currently not able to upload a volume"},{"line_number":7,"context_line":"    that is based on an image back to glance. This happens"},{"line_number":8,"context_line":"    if glance multistore is enabled. When enabling multistore,"},{"line_number":9,"context_line":"    the properties \"os_glance_failed_import\" and"},{"line_number":10,"context_line":"    \"os_glance_importing_to_stores\" will be stored as metadata"},{"line_number":11,"context_line":"    in the cinder volume created from the glance image."},{"line_number":12,"context_line":"    These metadata will cause problems when later, it is"},{"line_number":13,"context_line":"    attempted to perform some actions with Glance, such as"},{"line_number":14,"context_line":"    uploading the volume."}],"source_content_type":"text/x-yaml","patch_set":18,"id":"9fd63f5a_1f597477","line":14,"range":{"start_line":6,"start_character":4,"end_line":14,"end_character":25},"in_reply_to":"ca43d7e6_117decb5","updated":"2022-11-10 20:01:27.000000000","message":"Done","commit_id":"7b5ee938b281810773882942045aa05e505f400f"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"17694ac4b4d8431228c5ab219656651d38123391","unresolved":true,"context_lines":[{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    We introduced a new config parameter,"},{"line_number":5,"context_line":"    ``reserved_image_namespaces``, that allows "},{"line_number":6,"context_line":"    operators to set the image properties to filter"},{"line_number":7,"context_line":"    out from volume image metadata by namespace when"},{"line_number":8,"context_line":"    performing operations like creating a bootable"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"29cc2f30_2de09233","line":5,"range":{"start_line":5,"start_character":46,"end_line":5,"end_character":47},"updated":"2022-11-11 13:48:24.000000000","message":"There seems to be an extra space here.","commit_id":"2c296ba642c0c20bfeefa82f6235ff5f326f41c3"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"7438129a446423215b97adb1ff52edef4d4d4239","unresolved":false,"context_lines":[{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    We introduced a new config parameter,"},{"line_number":5,"context_line":"    ``reserved_image_namespaces``, that allows "},{"line_number":6,"context_line":"    operators to set the image properties to filter"},{"line_number":7,"context_line":"    out from volume image metadata by namespace when"},{"line_number":8,"context_line":"    performing operations like creating a bootable"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"be5f68d6_0afe946c","line":5,"range":{"start_line":5,"start_character":46,"end_line":5,"end_character":47},"in_reply_to":"29cc2f30_2de09233","updated":"2022-11-11 13:58:32.000000000","message":"Done","commit_id":"2c296ba642c0c20bfeefa82f6235ff5f326f41c3"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"abe15d667661b5e29b42d4aa2d4778884b6dba28","unresolved":true,"context_lines":[{"line_number":4,"context_line":"    We introduced a new config parameter,"},{"line_number":5,"context_line":"    ``reserved_image_namespaces``, that allows"},{"line_number":6,"context_line":"    operators to set the image properties to filter"},{"line_number":7,"context_line":"    out from volume image metadata by namespace when"},{"line_number":8,"context_line":"    performing operations like creating a bootable"},{"line_number":9,"context_line":"    volume or uploading a volume to Glance. These"},{"line_number":10,"context_line":"    properties, if not filtered out, causes failures"},{"line_number":11,"context_line":"    in further volume operations like uploading the"},{"line_number":12,"context_line":"    volume to image. The default value is ``[\u0027os_glance\u0027]``."}],"source_content_type":"text/x-yaml","patch_set":23,"id":"ca56705d_9a05e974","line":9,"range":{"start_line":7,"start_character":48,"end_line":9,"end_character":10},"updated":"2022-12-07 14:50:40.000000000","message":"maybe this needs to be reworded/removed, as we\u0027ve agreed to not remove the properties in case it is not uploading back to glance","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"5522f3fe554b0adb619011227c161e04e0b1cb98","unresolved":false,"context_lines":[{"line_number":4,"context_line":"    We introduced a new config parameter,"},{"line_number":5,"context_line":"    ``reserved_image_namespaces``, that allows"},{"line_number":6,"context_line":"    operators to set the image properties to filter"},{"line_number":7,"context_line":"    out from volume image metadata by namespace when"},{"line_number":8,"context_line":"    performing operations like creating a bootable"},{"line_number":9,"context_line":"    volume or uploading a volume to Glance. These"},{"line_number":10,"context_line":"    properties, if not filtered out, causes failures"},{"line_number":11,"context_line":"    in further volume operations like uploading the"},{"line_number":12,"context_line":"    volume to image. The default value is ``[\u0027os_glance\u0027]``."}],"source_content_type":"text/x-yaml","patch_set":23,"id":"daffaecb_161c978a","line":9,"range":{"start_line":7,"start_character":48,"end_line":9,"end_character":10},"in_reply_to":"ca56705d_9a05e974","updated":"2022-12-08 14:26:44.000000000","message":"Done","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"abe15d667661b5e29b42d4aa2d4778884b6dba28","unresolved":true,"context_lines":[{"line_number":8,"context_line":"    performing operations like creating a bootable"},{"line_number":9,"context_line":"    volume or uploading a volume to Glance. These"},{"line_number":10,"context_line":"    properties, if not filtered out, causes failures"},{"line_number":11,"context_line":"    in further volume operations like uploading the"},{"line_number":12,"context_line":"    volume to image. The default value is ``[\u0027os_glance\u0027]``."},{"line_number":13,"context_line":"fixes:"},{"line_number":14,"context_line":"  - |"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"747b9f41_4570e32f","line":11,"range":{"start_line":11,"start_character":4,"end_line":11,"end_character":37},"updated":"2022-12-07 14:50:40.000000000","message":"same as above","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"5522f3fe554b0adb619011227c161e04e0b1cb98","unresolved":false,"context_lines":[{"line_number":8,"context_line":"    performing operations like creating a bootable"},{"line_number":9,"context_line":"    volume or uploading a volume to Glance. These"},{"line_number":10,"context_line":"    properties, if not filtered out, causes failures"},{"line_number":11,"context_line":"    in further volume operations like uploading the"},{"line_number":12,"context_line":"    volume to image. The default value is ``[\u0027os_glance\u0027]``."},{"line_number":13,"context_line":"fixes:"},{"line_number":14,"context_line":"  - |"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"b4e9aa72_34f15a1f","line":11,"range":{"start_line":11,"start_character":4,"end_line":11,"end_character":37},"in_reply_to":"747b9f41_4570e32f","updated":"2022-12-08 14:26:44.000000000","message":"Done","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"abe15d667661b5e29b42d4aa2d4778884b6dba28","unresolved":true,"context_lines":[{"line_number":20,"context_line":"    a configuration option, ``reserved_image_namespaces``, that enables"},{"line_number":21,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":22,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":23,"context_line":"    image or download an image.  The default value is ``[\u0027os_glance\u0027]``."}],"source_content_type":"text/x-yaml","patch_set":23,"id":"53c51fcc_cd320894","line":23,"range":{"start_line":23,"start_character":10,"end_line":23,"end_character":30},"updated":"2022-12-07 14:50:40.000000000","message":"same as above","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"5522f3fe554b0adb619011227c161e04e0b1cb98","unresolved":false,"context_lines":[{"line_number":20,"context_line":"    a configuration option, ``reserved_image_namespaces``, that enables"},{"line_number":21,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":22,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":23,"context_line":"    image or download an image.  The default value is ``[\u0027os_glance\u0027]``."}],"source_content_type":"text/x-yaml","patch_set":23,"id":"e4f002c4_33ff82fa","line":23,"range":{"start_line":23,"start_character":10,"end_line":23,"end_character":30},"in_reply_to":"53c51fcc_cd320894","updated":"2022-12-08 14:26:44.000000000","message":"Done","commit_id":"df32a8cd62a736a5fe1cd65c4b5a787eaea74616"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b2f3426bfc7309e9bfe0ecce42721e4c8c664e23","unresolved":true,"context_lines":[{"line_number":4,"context_line":"    We introduced a new config parameter, ``reserved_image_namespaces``,"},{"line_number":5,"context_line":"    that allows operators to set the image properties to filter out from"},{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures in Glance upload."},{"line_number":8,"context_line":"fixes:"},{"line_number":9,"context_line":"  - |"},{"line_number":10,"context_line":"    `Bug #1945500 \u003chttps://bugs.launchpad.net/cinder/+bug/1945500\u003e`_: Fixed"}],"source_content_type":"text/x-yaml","patch_set":28,"id":"12c654e4_628fb328","line":7,"range":{"start_line":7,"start_character":58,"end_line":7,"end_character":75},"updated":"2022-12-20 17:22:24.000000000","message":"on the glance side.","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"ae22d464c0f7428a48f181add1322202d1f0dfd2","unresolved":false,"context_lines":[{"line_number":4,"context_line":"    We introduced a new config parameter, ``reserved_image_namespaces``,"},{"line_number":5,"context_line":"    that allows operators to set the image properties to filter out from"},{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures in Glance upload."},{"line_number":8,"context_line":"fixes:"},{"line_number":9,"context_line":"  - |"},{"line_number":10,"context_line":"    `Bug #1945500 \u003chttps://bugs.launchpad.net/cinder/+bug/1945500\u003e`_: Fixed"}],"source_content_type":"text/x-yaml","patch_set":28,"id":"7fad8c52_fca4d486","line":7,"range":{"start_line":7,"start_character":58,"end_line":7,"end_character":75},"in_reply_to":"12c654e4_628fb328","updated":"2022-12-21 11:26:43.000000000","message":"Done","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"cf47ae1d218e3b0e93a5b2ff3599671632eed9c7","unresolved":true,"context_lines":[{"line_number":15,"context_line":"    a configuration option, ``reserved_image_namespaces``, that enables"},{"line_number":16,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":17,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":18,"context_line":"    image or download an image. Moreover, we hardcoded the filter for"},{"line_number":19,"context_line":"    ``os_glance`` namespace."}],"source_content_type":"text/x-yaml","patch_set":28,"id":"9f0758b3_f0683688","line":18,"range":{"start_line":18,"start_character":13,"end_line":18,"end_character":30},"updated":"2023-01-25 15:29:39.000000000","message":"I believe you missed removing this","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b2f3426bfc7309e9bfe0ecce42721e4c8c664e23","unresolved":true,"context_lines":[{"line_number":15,"context_line":"    a configuration option, ``reserved_image_namespaces``, that enables"},{"line_number":16,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":17,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":18,"context_line":"    image or download an image. Moreover, we hardcoded the filter for"},{"line_number":19,"context_line":"    ``os_glance`` namespace."}],"source_content_type":"text/x-yaml","patch_set":28,"id":"7398cd9a_286663fd","line":18,"range":{"start_line":18,"start_character":13,"end_line":18,"end_character":30},"updated":"2022-12-20 17:22:24.000000000","message":"I don\u0027t see code for doing this on image download and we don\u0027t actually want to do that right? because we want to preserve all properties and only filter when needed.\nIf that\u0027s the case, we can remove this part.","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"ae22d464c0f7428a48f181add1322202d1f0dfd2","unresolved":false,"context_lines":[{"line_number":15,"context_line":"    a configuration option, ``reserved_image_namespaces``, that enables"},{"line_number":16,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":17,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":18,"context_line":"    image or download an image. Moreover, we hardcoded the filter for"},{"line_number":19,"context_line":"    ``os_glance`` namespace."}],"source_content_type":"text/x-yaml","patch_set":28,"id":"83a5e19f_2f02bf15","line":18,"range":{"start_line":18,"start_character":13,"end_line":18,"end_character":30},"in_reply_to":"7398cd9a_286663fd","updated":"2022-12-21 11:26:43.000000000","message":"Done","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"5beffbc9a166b431fab4481aeae5de5c4ae77ec2","unresolved":false,"context_lines":[{"line_number":15,"context_line":"    a configuration option, ``reserved_image_namespaces``, that enables"},{"line_number":16,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":17,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":18,"context_line":"    image or download an image. Moreover, we hardcoded the filter for"},{"line_number":19,"context_line":"    ``os_glance`` namespace."}],"source_content_type":"text/x-yaml","patch_set":28,"id":"a3d0380b_1ee73a9b","line":18,"range":{"start_line":18,"start_character":13,"end_line":18,"end_character":30},"in_reply_to":"9f0758b3_f0683688","updated":"2023-01-25 15:30:12.000000000","message":"disregard this, it was an old draft.","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b2f3426bfc7309e9bfe0ecce42721e4c8c664e23","unresolved":true,"context_lines":[{"line_number":16,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":17,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":18,"context_line":"    image or download an image. Moreover, we hardcoded the filter for"},{"line_number":19,"context_line":"    ``os_glance`` namespace."}],"source_content_type":"text/x-yaml","patch_set":28,"id":"67f0d9b6_003bc973","line":19,"range":{"start_line":19,"start_character":6,"end_line":19,"end_character":15},"updated":"2022-12-20 17:22:24.000000000","message":"``os_glance`` and ``img_signature``","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"ae22d464c0f7428a48f181add1322202d1f0dfd2","unresolved":false,"context_lines":[{"line_number":16,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":17,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":18,"context_line":"    image or download an image. Moreover, we hardcoded the filter for"},{"line_number":19,"context_line":"    ``os_glance`` namespace."}],"source_content_type":"text/x-yaml","patch_set":28,"id":"c3e4f015_b3090703","line":19,"range":{"start_line":19,"start_character":6,"end_line":19,"end_character":15},"in_reply_to":"67f0d9b6_003bc973","updated":"2022-12-21 11:26:43.000000000","message":"Done","commit_id":"cefff2d9824a0e76e0635f375cec3b8366e5a221"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"9c90ae4e007b9abd3d47f5bb32f5937b048b6fbb","unresolved":true,"context_lines":[{"line_number":4,"context_line":"    We introduced a new config parameter, ``reserved_image_namespaces``,"},{"line_number":5,"context_line":"    that allows operators to set the image properties to filter out from"},{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures when uploading "},{"line_number":8,"context_line":"    images back to Glance. The error will happen on Glance side when the "},{"line_number":9,"context_line":"    reserved namespaces are used."},{"line_number":10,"context_line":"fixes:"}],"source_content_type":"text/x-yaml","patch_set":29,"id":"446c7044_2f1f9434","line":7,"range":{"start_line":7,"start_character":72,"end_line":7,"end_character":73},"updated":"2022-12-21 18:00:37.000000000","message":"nit: extra whitespace","commit_id":"9726c315e29cbdde703b56cf4615e40965a4f0af"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6b1d70dfcddfd8e60372b896b39a8206b44c0f05","unresolved":false,"context_lines":[{"line_number":4,"context_line":"    We introduced a new config parameter, ``reserved_image_namespaces``,"},{"line_number":5,"context_line":"    that allows operators to set the image properties to filter out from"},{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures when uploading "},{"line_number":8,"context_line":"    images back to Glance. The error will happen on Glance side when the "},{"line_number":9,"context_line":"    reserved namespaces are used."},{"line_number":10,"context_line":"fixes:"}],"source_content_type":"text/x-yaml","patch_set":29,"id":"4b0820a3_93b7044e","line":7,"range":{"start_line":7,"start_character":72,"end_line":7,"end_character":73},"in_reply_to":"446c7044_2f1f9434","updated":"2022-12-21 18:02:42.000000000","message":"Done","commit_id":"9726c315e29cbdde703b56cf4615e40965a4f0af"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"9c90ae4e007b9abd3d47f5bb32f5937b048b6fbb","unresolved":true,"context_lines":[{"line_number":5,"context_line":"    that allows operators to set the image properties to filter out from"},{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures when uploading "},{"line_number":8,"context_line":"    images back to Glance. The error will happen on Glance side when the "},{"line_number":9,"context_line":"    reserved namespaces are used."},{"line_number":10,"context_line":"fixes:"},{"line_number":11,"context_line":"  - |"}],"source_content_type":"text/x-yaml","patch_set":29,"id":"10d0be13_7d90f94a","line":8,"range":{"start_line":8,"start_character":72,"end_line":8,"end_character":73},"updated":"2022-12-21 18:00:37.000000000","message":"nit: extra whitespace","commit_id":"9726c315e29cbdde703b56cf4615e40965a4f0af"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6b1d70dfcddfd8e60372b896b39a8206b44c0f05","unresolved":false,"context_lines":[{"line_number":5,"context_line":"    that allows operators to set the image properties to filter out from"},{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures when uploading "},{"line_number":8,"context_line":"    images back to Glance. The error will happen on Glance side when the "},{"line_number":9,"context_line":"    reserved namespaces are used."},{"line_number":10,"context_line":"fixes:"},{"line_number":11,"context_line":"  - |"}],"source_content_type":"text/x-yaml","patch_set":29,"id":"1e384198_7828b20c","line":8,"range":{"start_line":8,"start_character":72,"end_line":8,"end_character":73},"in_reply_to":"10d0be13_7d90f94a","updated":"2022-12-21 18:02:42.000000000","message":"Done","commit_id":"9726c315e29cbdde703b56cf4615e40965a4f0af"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"9c90ae4e007b9abd3d47f5bb32f5937b048b6fbb","unresolved":true,"context_lines":[{"line_number":17,"context_line":"    a configuration option, ``reserved_image_namespaces``, that enables"},{"line_number":18,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":19,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":20,"context_line":"    image. Moreover, we hardcoded the filter for ``os_glance`` and "},{"line_number":21,"context_line":"    ``img_signature`` namespace."}],"source_content_type":"text/x-yaml","patch_set":29,"id":"bdf6e8d4_807fd236","line":20,"range":{"start_line":20,"start_character":66,"end_line":20,"end_character":67},"updated":"2022-12-21 18:00:37.000000000","message":"nit: extra whitespace","commit_id":"9726c315e29cbdde703b56cf4615e40965a4f0af"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6b1d70dfcddfd8e60372b896b39a8206b44c0f05","unresolved":false,"context_lines":[{"line_number":17,"context_line":"    a configuration option, ``reserved_image_namespaces``, that enables"},{"line_number":18,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":19,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":20,"context_line":"    image. Moreover, we hardcoded the filter for ``os_glance`` and "},{"line_number":21,"context_line":"    ``img_signature`` namespace."}],"source_content_type":"text/x-yaml","patch_set":29,"id":"422a4cdc_138cb733","line":20,"range":{"start_line":20,"start_character":66,"end_line":20,"end_character":67},"in_reply_to":"bdf6e8d4_807fd236","updated":"2022-12-21 18:02:42.000000000","message":"Done","commit_id":"9726c315e29cbdde703b56cf4615e40965a4f0af"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fa925e6ffa5c6679115586578aa6bdad7154fc45","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    We introduced a new config parameter, ``reserved_image_namespaces``,"},{"line_number":5,"context_line":"    that allows operators to set the image properties to filter out from"},{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures when uploading"},{"line_number":8,"context_line":"    images back to Glance. The error will happen on Glance side when the"},{"line_number":9,"context_line":"    reserved namespaces are used."},{"line_number":10,"context_line":"fixes:"},{"line_number":11,"context_line":"  - |"},{"line_number":12,"context_line":"    `Bug #1945500 \u003chttps://bugs.launchpad.net/cinder/+bug/1945500\u003e`_: Fixed"}],"source_content_type":"text/x-yaml","patch_set":32,"id":"0e0715a0_f5df2442","line":9,"range":{"start_line":4,"start_character":0,"end_line":9,"end_character":33},"updated":"2023-01-04 21:43:00.000000000","message":"Maybe add something like \"This option is useful when an operator wants to use the Glance property protections feature to make some image properties read-only.\"","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"d979e8d65509b92ebdbc029c9ce362301df1c412","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    We introduced a new config parameter, ``reserved_image_namespaces``,"},{"line_number":5,"context_line":"    that allows operators to set the image properties to filter out from"},{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures when uploading"},{"line_number":8,"context_line":"    images back to Glance. The error will happen on Glance side when the"},{"line_number":9,"context_line":"    reserved namespaces are used."},{"line_number":10,"context_line":"fixes:"},{"line_number":11,"context_line":"  - |"},{"line_number":12,"context_line":"    `Bug #1945500 \u003chttps://bugs.launchpad.net/cinder/+bug/1945500\u003e`_: Fixed"}],"source_content_type":"text/x-yaml","patch_set":32,"id":"51012ebd_466e6f02","line":9,"range":{"start_line":4,"start_character":0,"end_line":9,"end_character":33},"in_reply_to":"0e0715a0_f5df2442","updated":"2023-01-05 16:33:00.000000000","message":"Done","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fa925e6ffa5c6679115586578aa6bdad7154fc45","unresolved":true,"context_lines":[{"line_number":13,"context_line":"    an error when uploading to Glance a previously downloaded glance image"},{"line_number":14,"context_line":"    when glance multistore is enabled. Glance reserves image properties"},{"line_number":15,"context_line":"    in the namespace \u0027os_glance\u0027 for its own use and will not allow"},{"line_number":16,"context_line":"    images to be created with these properties. This fix introduces"},{"line_number":17,"context_line":"    a configuration option, ``reserved_image_namespaces``, that enables"},{"line_number":18,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":19,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":20,"context_line":"    image. Moreover, we hardcoded the filter for ``os_glance`` and"},{"line_number":21,"context_line":"    ``img_signature`` namespace."}],"source_content_type":"text/x-yaml","patch_set":32,"id":"4bfcfd6a_71f0d121","line":21,"range":{"start_line":16,"start_character":48,"end_line":21,"end_character":32},"updated":"2023-01-04 21:43:00.000000000","message":"I suggest changing this to something like:\n\n  Additionally, there are image properties, such as those associated with image signature verification, that are stored in a volume\u0027s image metadata, but which don\u0027t make sense to add to a new image when a volume is being uploaded as an image.  Thus Cinder will no longer include any volume image metadata in the namespaces ``os_glance`` and ``img_signature`` when it creates an image in Glance.  Because the Glance property protections feature allows an operator to configure specific image properties as read-only, this fix adds a configuration option, ``reserved_image_namespaces``, that allows an operator to exclude additional image properties by namespace (the ``os_glance`` and ``img_signature`` namespaces are *always* excluded).","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"d979e8d65509b92ebdbc029c9ce362301df1c412","unresolved":false,"context_lines":[{"line_number":13,"context_line":"    an error when uploading to Glance a previously downloaded glance image"},{"line_number":14,"context_line":"    when glance multistore is enabled. Glance reserves image properties"},{"line_number":15,"context_line":"    in the namespace \u0027os_glance\u0027 for its own use and will not allow"},{"line_number":16,"context_line":"    images to be created with these properties. This fix introduces"},{"line_number":17,"context_line":"    a configuration option, ``reserved_image_namespaces``, that enables"},{"line_number":18,"context_line":"    operators to specify a list of image property namespaces that will"},{"line_number":19,"context_line":"    not be included when Cinder is requested to upload a volume as an"},{"line_number":20,"context_line":"    image. Moreover, we hardcoded the filter for ``os_glance`` and"},{"line_number":21,"context_line":"    ``img_signature`` namespace."}],"source_content_type":"text/x-yaml","patch_set":32,"id":"5d78cb35_46989d53","line":21,"range":{"start_line":16,"start_character":48,"end_line":21,"end_character":32},"in_reply_to":"4bfcfd6a_71f0d121","updated":"2023-01-05 16:33:00.000000000","message":"Done","commit_id":"03c944e6c5e88d3f5522e47ee81a8887986d0382"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4b383187a4251b100a7d0a90d35201fc238af4b5","unresolved":true,"context_lines":[{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures when uploading"},{"line_number":8,"context_line":"    images back to Glance. The error will happen on Glance side when the"},{"line_number":9,"context_line":"    reserved namespaces are used. This option is also useful when an operator "},{"line_number":10,"context_line":"    wants to use the Glance property protections feature to make some image"},{"line_number":11,"context_line":"    properties read-only."},{"line_number":12,"context_line":"fixes:"}],"source_content_type":"text/x-yaml","patch_set":37,"id":"8fcf090d_f678a7c5","line":9,"range":{"start_line":9,"start_character":77,"end_line":9,"end_character":78},"updated":"2023-01-27 14:39:40.000000000","message":"nit: remove this","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4b383187a4251b100a7d0a90d35201fc238af4b5","unresolved":true,"context_lines":[{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures when uploading"},{"line_number":8,"context_line":"    images back to Glance. The error will happen on Glance side when the"},{"line_number":9,"context_line":"    reserved namespaces are used. This option is also useful when an operator "},{"line_number":10,"context_line":"    wants to use the Glance property protections feature to make some image"},{"line_number":11,"context_line":"    properties read-only."},{"line_number":12,"context_line":"fixes:"}],"source_content_type":"text/x-yaml","patch_set":37,"id":"def1f1cb_951030fe","line":9,"range":{"start_line":9,"start_character":77,"end_line":9,"end_character":78},"updated":"2023-01-27 14:39:40.000000000","message":"remove this","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"0af7bd649a9ee179d420dc1e716f5f3d03c6b6f0","unresolved":false,"context_lines":[{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures when uploading"},{"line_number":8,"context_line":"    images back to Glance. The error will happen on Glance side when the"},{"line_number":9,"context_line":"    reserved namespaces are used. This option is also useful when an operator "},{"line_number":10,"context_line":"    wants to use the Glance property protections feature to make some image"},{"line_number":11,"context_line":"    properties read-only."},{"line_number":12,"context_line":"fixes:"}],"source_content_type":"text/x-yaml","patch_set":37,"id":"0434e177_b111e189","line":9,"range":{"start_line":9,"start_character":77,"end_line":9,"end_character":78},"in_reply_to":"8fcf090d_f678a7c5","updated":"2023-01-27 18:02:53.000000000","message":"Done","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"0af7bd649a9ee179d420dc1e716f5f3d03c6b6f0","unresolved":false,"context_lines":[{"line_number":6,"context_line":"    volume image metadata by namespace when uploading a volume to Glance."},{"line_number":7,"context_line":"    These properties, if not filtered out, cause failures when uploading"},{"line_number":8,"context_line":"    images back to Glance. The error will happen on Glance side when the"},{"line_number":9,"context_line":"    reserved namespaces are used. This option is also useful when an operator "},{"line_number":10,"context_line":"    wants to use the Glance property protections feature to make some image"},{"line_number":11,"context_line":"    properties read-only."},{"line_number":12,"context_line":"fixes:"}],"source_content_type":"text/x-yaml","patch_set":37,"id":"0510e75c_ea0a83a6","line":9,"range":{"start_line":9,"start_character":77,"end_line":9,"end_character":78},"in_reply_to":"def1f1cb_951030fe","updated":"2023-01-27 18:02:53.000000000","message":"Done","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4b383187a4251b100a7d0a90d35201fc238af4b5","unresolved":true,"context_lines":[{"line_number":17,"context_line":"    in the namespace \u0027os_glance\u0027 for its own use and will not allow"},{"line_number":18,"context_line":"    images to be created with these properties. Additionally, there are image"},{"line_number":19,"context_line":"    properties, such as those associated with image signature verification,"},{"line_number":20,"context_line":"    that are stored in a volume\u0027s image metadata, which should not be added "},{"line_number":21,"context_line":"    to a new image when a volume is being uploaded as an image. Thus Cinder"},{"line_number":22,"context_line":"    will no longer include any volume image metadata in the namespaces"},{"line_number":23,"context_line":"    ``os_glance`` and ``img_signature`` when it creates an image in Glance."}],"source_content_type":"text/x-yaml","patch_set":37,"id":"a5e3ab1a_63332c9c","line":20,"range":{"start_line":20,"start_character":75,"end_line":20,"end_character":76},"updated":"2023-01-27 14:39:40.000000000","message":"nit: remove this","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4b383187a4251b100a7d0a90d35201fc238af4b5","unresolved":true,"context_lines":[{"line_number":17,"context_line":"    in the namespace \u0027os_glance\u0027 for its own use and will not allow"},{"line_number":18,"context_line":"    images to be created with these properties. Additionally, there are image"},{"line_number":19,"context_line":"    properties, such as those associated with image signature verification,"},{"line_number":20,"context_line":"    that are stored in a volume\u0027s image metadata, which should not be added "},{"line_number":21,"context_line":"    to a new image when a volume is being uploaded as an image. Thus Cinder"},{"line_number":22,"context_line":"    will no longer include any volume image metadata in the namespaces"},{"line_number":23,"context_line":"    ``os_glance`` and ``img_signature`` when it creates an image in Glance."}],"source_content_type":"text/x-yaml","patch_set":37,"id":"b67b2b3d_ec0a2d0d","line":20,"range":{"start_line":20,"start_character":75,"end_line":20,"end_character":76},"updated":"2023-01-27 14:39:40.000000000","message":"remove this","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"0af7bd649a9ee179d420dc1e716f5f3d03c6b6f0","unresolved":false,"context_lines":[{"line_number":17,"context_line":"    in the namespace \u0027os_glance\u0027 for its own use and will not allow"},{"line_number":18,"context_line":"    images to be created with these properties. Additionally, there are image"},{"line_number":19,"context_line":"    properties, such as those associated with image signature verification,"},{"line_number":20,"context_line":"    that are stored in a volume\u0027s image metadata, which should not be added "},{"line_number":21,"context_line":"    to a new image when a volume is being uploaded as an image. Thus Cinder"},{"line_number":22,"context_line":"    will no longer include any volume image metadata in the namespaces"},{"line_number":23,"context_line":"    ``os_glance`` and ``img_signature`` when it creates an image in Glance."}],"source_content_type":"text/x-yaml","patch_set":37,"id":"850d0f24_7fb9c981","line":20,"range":{"start_line":20,"start_character":75,"end_line":20,"end_character":76},"in_reply_to":"a5e3ab1a_63332c9c","updated":"2023-01-27 18:02:53.000000000","message":"Done","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"0af7bd649a9ee179d420dc1e716f5f3d03c6b6f0","unresolved":false,"context_lines":[{"line_number":17,"context_line":"    in the namespace \u0027os_glance\u0027 for its own use and will not allow"},{"line_number":18,"context_line":"    images to be created with these properties. Additionally, there are image"},{"line_number":19,"context_line":"    properties, such as those associated with image signature verification,"},{"line_number":20,"context_line":"    that are stored in a volume\u0027s image metadata, which should not be added "},{"line_number":21,"context_line":"    to a new image when a volume is being uploaded as an image. Thus Cinder"},{"line_number":22,"context_line":"    will no longer include any volume image metadata in the namespaces"},{"line_number":23,"context_line":"    ``os_glance`` and ``img_signature`` when it creates an image in Glance."}],"source_content_type":"text/x-yaml","patch_set":37,"id":"62c5bc11_b176b0cd","line":20,"range":{"start_line":20,"start_character":75,"end_line":20,"end_character":76},"in_reply_to":"b67b2b3d_ec0a2d0d","updated":"2023-01-27 18:02:53.000000000","message":"Done","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4b383187a4251b100a7d0a90d35201fc238af4b5","unresolved":true,"context_lines":[{"line_number":21,"context_line":"    to a new image when a volume is being uploaded as an image. Thus Cinder"},{"line_number":22,"context_line":"    will no longer include any volume image metadata in the namespaces"},{"line_number":23,"context_line":"    ``os_glance`` and ``img_signature`` when it creates an image in Glance."},{"line_number":24,"context_line":"    Furthermore, because the Glance property protections feature allows an "},{"line_number":25,"context_line":"    operator to configure specific image properties as read-only, this fix"},{"line_number":26,"context_line":"    adds a configuration option, ``reserved_image_namespaces``, that allows an"},{"line_number":27,"context_line":"    operator to exclude additional image properties by namespace (the"}],"source_content_type":"text/x-yaml","patch_set":37,"id":"d2cd1e8a_3ebc1f78","line":24,"range":{"start_line":24,"start_character":74,"end_line":24,"end_character":75},"updated":"2023-01-27 14:39:40.000000000","message":"nit: remove this","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4b383187a4251b100a7d0a90d35201fc238af4b5","unresolved":true,"context_lines":[{"line_number":21,"context_line":"    to a new image when a volume is being uploaded as an image. Thus Cinder"},{"line_number":22,"context_line":"    will no longer include any volume image metadata in the namespaces"},{"line_number":23,"context_line":"    ``os_glance`` and ``img_signature`` when it creates an image in Glance."},{"line_number":24,"context_line":"    Furthermore, because the Glance property protections feature allows an "},{"line_number":25,"context_line":"    operator to configure specific image properties as read-only, this fix"},{"line_number":26,"context_line":"    adds a configuration option, ``reserved_image_namespaces``, that allows an"},{"line_number":27,"context_line":"    operator to exclude additional image properties by namespace (the"}],"source_content_type":"text/x-yaml","patch_set":37,"id":"c84a86d7_95582ac3","line":24,"range":{"start_line":24,"start_character":74,"end_line":24,"end_character":75},"updated":"2023-01-27 14:39:40.000000000","message":"remove this","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"0af7bd649a9ee179d420dc1e716f5f3d03c6b6f0","unresolved":false,"context_lines":[{"line_number":21,"context_line":"    to a new image when a volume is being uploaded as an image. Thus Cinder"},{"line_number":22,"context_line":"    will no longer include any volume image metadata in the namespaces"},{"line_number":23,"context_line":"    ``os_glance`` and ``img_signature`` when it creates an image in Glance."},{"line_number":24,"context_line":"    Furthermore, because the Glance property protections feature allows an "},{"line_number":25,"context_line":"    operator to configure specific image properties as read-only, this fix"},{"line_number":26,"context_line":"    adds a configuration option, ``reserved_image_namespaces``, that allows an"},{"line_number":27,"context_line":"    operator to exclude additional image properties by namespace (the"}],"source_content_type":"text/x-yaml","patch_set":37,"id":"32a5405d_3305218a","line":24,"range":{"start_line":24,"start_character":74,"end_line":24,"end_character":75},"in_reply_to":"c84a86d7_95582ac3","updated":"2023-01-27 18:02:53.000000000","message":"Done","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"},{"author":{"_account_id":28356,"name":"Rafael Weingartner","email":"rafael@apache.org","username":"rafaelweingartner"},"change_message_id":"0af7bd649a9ee179d420dc1e716f5f3d03c6b6f0","unresolved":false,"context_lines":[{"line_number":21,"context_line":"    to a new image when a volume is being uploaded as an image. Thus Cinder"},{"line_number":22,"context_line":"    will no longer include any volume image metadata in the namespaces"},{"line_number":23,"context_line":"    ``os_glance`` and ``img_signature`` when it creates an image in Glance."},{"line_number":24,"context_line":"    Furthermore, because the Glance property protections feature allows an "},{"line_number":25,"context_line":"    operator to configure specific image properties as read-only, this fix"},{"line_number":26,"context_line":"    adds a configuration option, ``reserved_image_namespaces``, that allows an"},{"line_number":27,"context_line":"    operator to exclude additional image properties by namespace (the"}],"source_content_type":"text/x-yaml","patch_set":37,"id":"b47ae1a7_2dd93f18","line":24,"range":{"start_line":24,"start_character":74,"end_line":24,"end_character":75},"in_reply_to":"d2cd1e8a_3ebc1f78","updated":"2023-01-27 18:02:53.000000000","message":"Done","commit_id":"a01329abbf79f6c405d34d29d1bfeff89ddc322d"}]}
