On 02/16/11 04:28 AM, Darren Kenny wrote:
On 16/02/2011 01:14, Keith Mitchell wrote:
On 02/15/11 09:47 AM, Karen Tung wrote:
Hi Keith,
The changes look good to me.
Your examples lead me to think of an usage example
that might be confusing to users.
If people want to log stdout and stderr, they have to
specify:
Popen(cmd, stdout=Popen.STORE, stderr=Popen.STORE, logger=mylogger)
if I just specify:
Popen(cmd, logger=mylogger)
nothing will get logged, and no warning is displayed saying why.
The above syntax is not very intuitive, in my opinion,
because if I didn't read the document on how to
use this API, it's natural to think that if I want to log something,
I just provide the logger and everything will be taken care of.
Can we at least display a warning if they specify a logger
but not stdout and stderr? I think it is even better to automatically
assign stdout and stderr to Popen.STORE if a logger is provided,
but those values are not set.
Hi Karen,
You bring up a very good point. In general, I'm trying to keep the validation
lax (I'd rather leave open a door to make a mistake than lock out a viable, if
rare, use case). However, in this case, while having either stdout *or* stderr
be set to None would be reasonable when using the logger, if *both* are None
(and logger is set), it's clearly an error. I'll update to raise a ValueError in
this case.
Is there a reason that we couldn't just automatically log the stdin/out to the
logger if None is specified for their values? Just seems like a nice thing to
do.
Thanks,
Darren.
I'm open to addressing this in the future, but for now, I'd like to see
how it plays out. It would take a certain amount of time and testing to
introduce the desired behavior, and could introduce bugs in edge cases
for certain combinations of arguments - there are legitimate reasons for
specifying 'None' as the target for stdout/stderr (which causes the
subprocess to inherit those handles from the parent), so it's not as
simple as just changing a "None" to a "Popen.PIPE" if logger is set.
- Keith
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
