)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"a59e9c755c86e0e78e89c49d9df4c3af29bcd784","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4ee43962_2eddd054","updated":"2024-10-25 14:44:18.000000000","message":"I didn\u0027t post my response before (was left here as draft), but AFAICT I\u0027ve handled the comments as best as I can? Did I miss something or do you have advice on how to resolve the other issues","commit_id":"fbe4d42637615f04c38690d7457067b5eba2fc04"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"2cc4ea25cbec759808e3d62f73d6fda6cb7e0b2f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"07cb4997_589e06cc","updated":"2024-10-24 07:46:44.000000000","message":"See comments on the previous patchset","commit_id":"fbe4d42637615f04c38690d7457067b5eba2fc04"}],"ironic_python_agent/disk_utils.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d614681d1ea8e43910de75e32e297bc671cec7dc","unresolved":false,"context_lines":[{"line_number":456,"context_line":"    return img_format, size"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"def populate_image(src, dst, out_format\u003d\u0027raw\u0027, conv_flags\u003dNone,"},{"line_number":460,"context_line":"                   is_raw\u003dFalse, sparse_size\u003d\u00270\u0027, **convert_args):"},{"line_number":461,"context_line":"    \"\"\"Populate a provided destination device with the image"},{"line_number":462,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"a81b9e9c_e69e265a","line":459,"updated":"2024-10-21 12:47:29.000000000","message":"Please add the new argument to the end of the list. While the chances of any downstream hardware managers using this function are slim, they are not zero.","commit_id":"8ba8cf7fdf97c2eac9a8a65bdfd1e0c1dd67ef62"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"a59e9c755c86e0e78e89c49d9df4c3af29bcd784","unresolved":false,"context_lines":[{"line_number":456,"context_line":"    return img_format, size"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"def populate_image(src, dst, out_format\u003d\u0027raw\u0027, conv_flags\u003dNone,"},{"line_number":460,"context_line":"                   is_raw\u003dFalse, sparse_size\u003d\u00270\u0027, **convert_args):"},{"line_number":461,"context_line":"    \"\"\"Populate a provided destination device with the image"},{"line_number":462,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"2f1a9c6d_f0a2d1f6","line":459,"in_reply_to":"a81b9e9c_e69e265a","updated":"2024-10-25 14:44:18.000000000","message":"This is not possible given the current layout of **convert_args being expected at the end. I will move it closer to the end. I\u0027d suggest -- maybe in a later change -- we look at removing convert_args altogether.\n\nI will move out_format to the end before convert_args, at least that should help.","commit_id":"8ba8cf7fdf97c2eac9a8a65bdfd1e0c1dd67ef62"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d614681d1ea8e43910de75e32e297bc671cec7dc","unresolved":false,"context_lines":[{"line_number":481,"context_line":"            dd(src, dst, conv_flags\u003dconv_flags)"},{"line_number":482,"context_line":"        else:"},{"line_number":483,"context_line":"            command \u003d [\u0027qemu-img\u0027, \u0027convert\u0027,"},{"line_number":484,"context_line":"                       \u0027-t\u0027, \u0027directsync\u0027, \u0027-S\u0027, \u00270\u0027, \u0027-O\u0027, \u0027host_device\u0027,"},{"line_number":485,"context_line":"                       \u0027-W\u0027]"},{"line_number":486,"context_line":"            if convert_args.get(\u0027source_format\u0027):"},{"line_number":487,"context_line":"                command +\u003d [\u0027-f\u0027, convert_args.get(\u0027source_format\u0027)]"}],"source_content_type":"text/x-python","patch_set":5,"id":"d2d9e332_8e3ca68b","line":484,"updated":"2024-10-21 12:47:29.000000000","message":"you meant out_format instead of \u0027host_device\u0027 (but see below)","commit_id":"8ba8cf7fdf97c2eac9a8a65bdfd1e0c1dd67ef62"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d614681d1ea8e43910de75e32e297bc671cec7dc","unresolved":false,"context_lines":[{"line_number":484,"context_line":"                       \u0027-t\u0027, \u0027directsync\u0027, \u0027-S\u0027, \u00270\u0027, \u0027-O\u0027, \u0027host_device\u0027,"},{"line_number":485,"context_line":"                       \u0027-W\u0027]"},{"line_number":486,"context_line":"            if convert_args.get(\u0027source_format\u0027):"},{"line_number":487,"context_line":"                command +\u003d [\u0027-f\u0027, convert_args.get(\u0027source_format\u0027)]"},{"line_number":488,"context_line":"            command +\u003d [src, dst]"},{"line_number":489,"context_line":"            LOG.info(\u0027Writing image with command: %s\u0027, \u0027 \u0027.join(command))"},{"line_number":490,"context_line":"            qemu_img.convert_image(src, dst,"}],"source_content_type":"text/x-python","patch_set":5,"id":"5c102459_c5ef41a6","line":487,"updated":"2024-10-21 12:47:29.000000000","message":"source_format should be an explicit argument to populate_image similarly to out_format (but see below)","commit_id":"8ba8cf7fdf97c2eac9a8a65bdfd1e0c1dd67ef62"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d614681d1ea8e43910de75e32e297bc671cec7dc","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            if convert_args.get(\u0027source_format\u0027):"},{"line_number":487,"context_line":"                command +\u003d [\u0027-f\u0027, convert_args.get(\u0027source_format\u0027)]"},{"line_number":488,"context_line":"            command +\u003d [src, dst]"},{"line_number":489,"context_line":"            LOG.info(\u0027Writing image with command: %s\u0027, \u0027 \u0027.join(command))"},{"line_number":490,"context_line":"            qemu_img.convert_image(src, dst,"},{"line_number":491,"context_line":"                                   out_format\u003dout_format,"},{"line_number":492,"context_line":"                                   run_as_root\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":5,"id":"6b4c2d67_d6d1ef9c","line":489,"updated":"2024-10-21 12:47:29.000000000","message":"It\u0027s probably not worth it to try compose a perfectly matching command for logging: utils.execute already logs everything that we run.","commit_id":"8ba8cf7fdf97c2eac9a8a65bdfd1e0c1dd67ef62"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"bdc6a34970096d4480863673a618454630db8391","unresolved":true,"context_lines":[{"line_number":457,"context_line":""},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"def populate_image(src, dst, conv_flags\u003dNone, is_raw\u003dFalse, sparse_size\u003d\u00270\u0027,"},{"line_number":460,"context_line":"                   source_format\u003dNone, out_format\u003d\u0027raw\u0027, **convert_args):"},{"line_number":461,"context_line":"    \"\"\"Populate a provided destination device with the image"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"    :param src: An image already security checked in format disk_format"}],"source_content_type":"text/x-python","patch_set":6,"id":"559a156d_e00a20fe","line":460,"updated":"2024-10-30 08:57:19.000000000","message":"I just cannot understand the need to reorder existing arguments...\n\nAlso, now that you don\u0027t use the argument explicitly, you don\u0027t actually need to move them out of convert_args (my comments very specifically referred to arguments that were used in the function body).","commit_id":"fbe4d42637615f04c38690d7457067b5eba2fc04"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"e485ba856caa7a860b9b5602cdf38d77a2eb935a","unresolved":true,"context_lines":[{"line_number":457,"context_line":""},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"def populate_image(src, dst, conv_flags\u003dNone, is_raw\u003dFalse, sparse_size\u003d\u00270\u0027,"},{"line_number":460,"context_line":"                   source_format\u003dNone, out_format\u003d\u0027raw\u0027, **convert_args):"},{"line_number":461,"context_line":"    \"\"\"Populate a provided destination device with the image"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"    :param src: An image already security checked in format disk_format"}],"source_content_type":"text/x-python","patch_set":6,"id":"eb1866d5_58724a7b","line":460,"in_reply_to":"559a156d_e00a20fe","updated":"2024-10-30 15:01:27.000000000","message":"so if I ordered the args:\n\nsrc, dest, conv_flags, source_format, is_raw, sparse_size, out_format, **convert_args\n\nthat satisfies your concerns? I\u0027m really confused as to your comment generally","commit_id":"fbe4d42637615f04c38690d7457067b5eba2fc04"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1cbda0d145b300f1aac2e715fb6a22f110fae896","unresolved":false,"context_lines":[{"line_number":457,"context_line":""},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"def populate_image(src, dst, conv_flags\u003dNone, is_raw\u003dFalse, sparse_size\u003d\u00270\u0027,"},{"line_number":460,"context_line":"                   source_format\u003dNone, out_format\u003d\u0027raw\u0027, **convert_args):"},{"line_number":461,"context_line":"    \"\"\"Populate a provided destination device with the image"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"    :param src: An image already security checked in format disk_format"}],"source_content_type":"text/x-python","patch_set":6,"id":"ccb6533d_d1f9470f","line":460,"in_reply_to":"eb1866d5_58724a7b","updated":"2024-11-04 17:41:26.000000000","message":"This works for me from the compatibility standpoint. Even better, since you no longer log the presumed command line:\n\n def populate_image(src, dst, conv_flags\u003dNone, **convert_args):","commit_id":"fbe4d42637615f04c38690d7457067b5eba2fc04"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1cbda0d145b300f1aac2e715fb6a22f110fae896","unresolved":true,"context_lines":[{"line_number":482,"context_line":"        else:"},{"line_number":483,"context_line":"            qemu_img.convert_image(src, dst,"},{"line_number":484,"context_line":"                                   out_format\u003dout_format,"},{"line_number":485,"context_line":"                                   run_as_root\u003dTrue,"},{"line_number":486,"context_line":"                                   sparse_size\u003dsparse_size,"},{"line_number":487,"context_line":"                                   source_format\u003dsource_format,"},{"line_number":488,"context_line":"                                   **convert_args)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5169e64e_90898494","line":485,"updated":"2024-11-04 17:41:26.000000000","message":"Side note: run_as_root needs to be killed with fire (my patch down the chain probably does it already)","commit_id":"c3e7f3becea659865c1db904f37b2c4c1a419cdf"}]}