Re: [PATCH] scripts/qcow2-to-stdout.py: Add script to write qcow2 images to stdout

[Alberto Garcia <berto@igalia.com>]

On Mon 01 Jul 2024 02:07:01 PM +03, Manos Pitsidianakis wrote:
>> and I'm not sure what benefits using pathlib brings in this case.
>
> implicit type requirement, argument value validations, path
> normalization etc.

Do you have a specific example? I don't see any difference in behavior
if I make input_file a pathlib.Path, I still need to check if the file
exists, etc., I don't see that this is validating anything.

>>     with open(raw_file, 'wb'):
>>         pass
>>
>> If so I don't see the benefit, I just need to create an empty file and
>> close it immediately.
>
> My only argument here is that it's "more pythonic" which I know is of
> little value and consequence :) Feel free to ignore! They were mere
> suggestions.

In general I would agree (that's why I'm opening files this way in other
parts of the script) but for this case I don't think it's worth it.

>> I'm not sure that I understand, why would I need to use a shell here?
>
> I must have meant capture_output=True, not shell=True, sorry for that
> 🤔. The explicit check=False says to the reader that this won't throw
> an exception so it's just for readability. The capture_output part is
> so that you can print the outputs if the return code is an error.

Ah I see.

I'll send a new version soon.

Thanks for the review,

Berto