On 12/04/2015 12:15 PM, Jan Cholasta wrote:
> On 4.12.2015 10:53, Petr Viktorin wrote:
>> On 12/04/2015 08:51 AM, Jan Cholasta wrote:
>>> On 3.12.2015 15:42, Petr Viktorin wrote:
>>>> On 12/03/2015 10:55 AM, Jan Cholasta wrote:
[...]
>>>>> If we do encode/decode in run(), I think there should be a way to
>>>>> override the default encoding. My idea was to either accept/return
>>>>> only
>>>>> bytes and let the caller handle encoding themselves, or to handle
>>>>> encoding in run(), but be able to override the defaults.
>>>>>
>>>>> Given we handle encoding in run(), I would imagine it like this:
>>>>>
>>>>>       def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
>>>>>               capture_stdout=False, skip_output=False, cwd=None,
>>>>>               runas=None, timeout=None, suplementary_groups=[],
>>>>>               capture_stderr=False, encode_stdout=False,
>>>>>               encode_stderr=False, encoding=None):
>>>>>
>>>>> i.e. nothing is captured by default, when stdout or stderr are
>>>>> captured
>>>>> they are returned as bytes by default, when stdout or stderr are
>>>>> returned as text they are decoded using the locale encoding by
>>>>> default,
>>>>> or the encoding can be explicitly specified.
>>>>>
>>>>> Do you think this is feasible?
>>>>
>>>> Feasible, yes.
>>>> In the majority of cases where the output is needed, we want text
>>>> encoded with locale.getpreferredencoding(), so I'd rather make that the
>>>> default rather than bytes.
>>>>
>>>> Or we could make "encode_stdout" imply "capture_stdout", so you
>>>> wouldn't
>>>> have to always specify both. (It would be better named "encoded_stdout"
>>>> in that case.)
>>>
>>> I think it should be OK to decode by default. (IMO it would be best to
>>> reverse the logic and name it "raw_stdout", with False default. Do we
>>> need "raw_stderr" at all? I don't think we do.)
>>
>> Actually, now that I think about it: if the result was a namedtuple
>> subclass, we can always set extra raw_stdout/raw_stderr attributes on
>> it. (Unless skip_output=True, of course.)
>>
>>     result = run(['generate_binary_data'])
>>     use(result.raw_stdout)
>>
>>     # and for backcompat,
>>     rc, _out, _err = result
> 
> Good idea.
> 
> Perhaps we should use the same names as CalledProcessError for the
> attributes, i.e. "returncode", "output", and "error_output",
> "raw_output", "raw_error_output" for the new attributes?

OK. It's also good to use different names than Popen's stdout/stderr,
which are file-like objects rather than strings. But then,
capture_stdout/capture_stderr should be capture_output/capture_error_output.

-- 
Petr Viktorin

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to