)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b687a7e72038a0592ad0f152e0869965a6485b5f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ff59f9fe_183a1348","updated":"2023-12-05 14:40:25.000000000","message":"You can fix the linter issues by running `pre-commit run -a`. However, I think there\u0027s more issues here. This should probably be marked as WIP right now?","commit_id":"5880cf658973aa5885d5d178e56e323e859a30c6"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"023a5e7b59e92792d3347ddbf5fbbb55e77b6bc7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4816fc0e_312ad2d5","in_reply_to":"6f81cd6b_66b21e54","updated":"2023-12-06 17:33:41.000000000","message":"Sorry, that was poorly worded/makes no sense. What I should have said is that it seems the Cinder implementation of this is still in-flight so this should probably be marked as WIP until that lands? However, you have correctly marked dependencies so that\u0027s probably sufficient.","commit_id":"5880cf658973aa5885d5d178e56e323e859a30c6"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"954e3c7ce957506eccc0bcb4c3cd54c278182792","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6f81cd6b_66b21e54","in_reply_to":"ff59f9fe_183a1348","updated":"2023-12-06 17:28:32.000000000","message":"Thank you for the tip with `pre-commit`. I\u0027ve updated the patchset accordingly.\n\n\u003e However, I think there\u0027s more issues here. This should probably be marked as WIP right now?\n\nAside from the questionable os-brick import what other issues are you seeing with this?","commit_id":"5880cf658973aa5885d5d178e56e323e859a30c6"}],"openstackclient/image/v2/image.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b687a7e72038a0592ad0f152e0869965a6485b5f","unresolved":true,"context_lines":[{"line_number":562,"context_line":""},{"line_number":563,"context_line":"                try:"},{"line_number":564,"context_line":"                    encryptor \u003d \\"},{"line_number":565,"context_line":"                        file_crypto.get_file_encryptor_for_image(kwargs)"},{"line_number":566,"context_line":"                    encryptor.encrypt("},{"line_number":567,"context_line":"                        encrypted_image_file,"},{"line_number":568,"context_line":"                        parsed_args.filename,"}],"source_content_type":"text/x-python","patch_set":1,"id":"23fa70aa_af8b15d5","line":565,"updated":"2023-12-05 14:40:25.000000000","message":"Do we need to pull in the entirety of os-brick for this? How many encryption backends are there? Could we vendor the code instead?","commit_id":"ad2e58e76ace505dce4ca634834691f9a5e1be1f"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"98a83f003834f807964c41f5ab75092327003c4f","unresolved":true,"context_lines":[{"line_number":562,"context_line":""},{"line_number":563,"context_line":"                try:"},{"line_number":564,"context_line":"                    encryptor \u003d \\"},{"line_number":565,"context_line":"                        file_crypto.get_file_encryptor_for_image(kwargs)"},{"line_number":566,"context_line":"                    encryptor.encrypt("},{"line_number":567,"context_line":"                        encrypted_image_file,"},{"line_number":568,"context_line":"                        parsed_args.filename,"}],"source_content_type":"text/x-python","patch_set":1,"id":"dc7c8f26_62ded021","line":565,"in_reply_to":"23fa70aa_af8b15d5","updated":"2023-12-06 17:08:54.000000000","message":"\u003e Do we need to pull in the entirety of os-brick for this?\n\nBack when the specs were drafted, Cinder asked to implement this in os-brick. For example see the logs here: https://meetings.opendev.org/meetings/image_encryption/2019/image_encryption.2019-08-12-13.00.log.html#l-59\nSo with the current agreements and specs yes, we\u0027d need to.\n\nPulling os-brick in for the openstackclient does not look like the perfect solution to me either though. However, we\u0027d need to agree on another spot with Cinder in case we want to change this. In my opinion, we should have a central place for this code to keep it maintainable and in sync for all its consumers. Maybe also Nova wants to join in on the fun of encrypted images in the future :)\n\n\u003e How many encryption backends are there?\n\nCurrently we offer one reference implementation driver right now (GnuPG symmetric). But the implementation is intentionally structured in a way that allows adding new encryption drivers easily.\n\n\u003e Could we vendor the code instead?\n\nWhat do you mean by \"vendor the code\" exactly?","commit_id":"ad2e58e76ace505dce4ca634834691f9a5e1be1f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dfcb7fe2d24b4c9ba809ec9c3326f4abb2b3b567","unresolved":true,"context_lines":[{"line_number":562,"context_line":""},{"line_number":563,"context_line":"                try:"},{"line_number":564,"context_line":"                    encryptor \u003d \\"},{"line_number":565,"context_line":"                        file_crypto.get_file_encryptor_for_image(kwargs)"},{"line_number":566,"context_line":"                    encryptor.encrypt("},{"line_number":567,"context_line":"                        encrypted_image_file,"},{"line_number":568,"context_line":"                        parsed_args.filename,"}],"source_content_type":"text/x-python","patch_set":1,"id":"4bc3210b_2f2df4bf","line":565,"in_reply_to":"dc7c8f26_62ded021","updated":"2023-12-06 17:37:49.000000000","message":"\u003e \u003e Do we need to pull in the entirety of os-brick for this?\n\u003e \n\u003e Back when the specs were drafted, Cinder asked to implement this in os-brick. For example see the logs here: https://meetings.opendev.org/meetings/image_encryption/2019/image_encryption.2019-08-12-13.00.log.html#l-59\n\u003e So with the current agreements and specs yes, we\u0027d need to.\n\nGotcha.\n\n\u003e Pulling os-brick in for the openstackclient does not look like the perfect solution to me either though. However, we\u0027d need to agree on another spot with Cinder in case we want to change this. In my opinion, we should have a central place for this code to keep it maintainable and in sync for all its consumers. Maybe also Nova wants to join in on the fun of encrypted images in the future :)\n\nIt really does feel like a separate library. I wouldn\u0027t be a fan of putting this into openstacksdk and to me, os-brick serves an existing purpose that is wholly different to this. I don\u0027t know what effort is involved in spinning up a new library, however, and my quick search didn\u0027t highlight any pre-existing libraries available for doing this.\n\n\u003e \u003e How many encryption backends are there?\n\u003e \n\u003e Currently we offer one reference implementation driver right now (GnuPG symmetric). But the implementation is intentionally structured in a way that allows adding new encryption drivers easily.\n\u003e \n\u003e \u003e Could we vendor the code instead?\n\u003e \n\u003e What do you mean by \"vendor the code\" exactly?\n\nCould we duplicate the code?","commit_id":"ad2e58e76ace505dce4ca634834691f9a5e1be1f"}]}