On 4.12.2015 12:43, Petr Viktorin wrote:
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?

(Actually, since "returncode" is not "return_code", it should probably be "returncode", "output", "erroroutput", "raw_output", "raw_erroroutput".)


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.

Works for me.

--
Jan Cholasta

--
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