Hi Karen,
Sue may answer more specifically, but from a general "philosophical"
standpoint, exceptions are for (generally unexpected) exceptional
conditions. So if a command is actually expected to have a non-zero
return, then I think it may be better to check the return code.
The best example may be to pretend we have a function to call "pkg
install <some pkg>", which returns 0 if it succeeds, 4 if the given pkg
is already installed on the system, and 1 for a failure to execute
(e.g., permissions error). So one might write a function "do_pkg_install":
def do_pkg_install(pkg):
cmd = ["pkg", "install", pkg]
pkg_popen = Popen(cmd, check_result=[0, 4])
if pkg_popen.returncode == 4:
print "Nothing to do"
else:
print pkg + " installed successfully!"
The expected (check_result) cases are handled gracefully, whereas
unexpected failures from permissions will lead to exceptions raised to
the caller, who can deal with them as needed.
Of course, the line between "expected failure" and "unexpected failure"
is a blurry one at times, so it's very context sensitive as to which
approach to take.
The other key point to make is that exceptions are used to *ensure* that
an error case that the local scope can't handle gets dealt with by the
caller (or its caller, or its caller, or...). If the error case is
either handled (or propagated/translated to a different exception) in
the local scope, then there's not as much benefit to the overhead of
generating the exception.
- Keith
On 02/14/11 11:52 AM, Karen Tung wrote:
Hi Sue,
Question: In all the Popen calls in create_client.py and
installadm.py, why not
utilize the check_result argument of the Popen() call?
I think that option in Popen() is provided to avoid these return code
checking.
Another nit in create_client.py and installadm.py is that the code have:
cmd = [CHECK_SETUP_SCRIPT]
ret = Popen(cmd).wait()
Why not just do:
ret = Popen([CHECK_SETUP_SCRIPT])
Thanks,
--Karen
On 02/14/11 11:25, Sue Sohn wrote:
Could I please get a review of the changes for:
7019438 Remove run_script from installadm
Webrev:
http://cr.opensolaris.org/~sohn/7019438
Thanks,
Sue
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
