[Tutor] Issue in using "subprocess.Popen" for parsing the command output

Cameron Simpson cs at cskk.id.au
Sun Nov 25 19:30:02 EST 2018


On 26Nov2018 09:03, Steven D'Aprano <steve at pearwood.info> wrote:
>On Sun, Nov 25, 2018 at 10:43:10PM +0530, srinivasan wrote:
>> 1. Am trying to improve the below code with "try" and "exception", 
>> could
>> you please help me how "try" and "exception" can be used on the below code
>> snippet. I hope in my code with try and exception, seems to be a bug.
>
>As a beginner, you should normally not use try...except to report
>errors. You should learn how to diagnose errors by reading the
>traceback. Covering up the traceback with try...except makes debugging
>harder.

Very true, but...

>Your use here:
>
>
>> *    try:*
>> *        cmd = "nmcli device wifi connect '%s' password '%s'" % (ssid, pw)*
>> *        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True, universal_newlines=True)*
>> *        stdout, stderr = proc.communicate()*
>> *        retcode = proc.returncode*
>> *        print("printing stdout!!!!!!!!!!", stdout)*
>> *        print("printing retcode!!!!!!!!!!", retcode)*
>> *    except subprocess.CalledProcessError as e:*
>> *        s = """While executing '{}' something went wrong.*
>> *                        Return code == '{}'*
>> *                        Return output:\n'{}'*
>> *                        """.format(cmd, e.returncode, e.output, shell=enable_shell)*
>> *        raise AssertionError(s)*
[...]
>But even if it doesn't fail, the next line:
>
>    raise AssertionError(s)
>
>is an abuse of exceptions. The failure here is *not* an assertion, and
>you shouldn't use AssertionError. You wouldn't use TypeError or
>UnicodeEncodeError or AttributeError. "AssertionError" should not be
>used for "some arbitrary error". [...]
>My opinion is, you should remove that try...except altogether. I don't
>think that it helps your code, even if it worked. Calls to Popen can
>fail in many, many ways, and it seems pointless to single out just one
>of them and to replace the useful traceback and error message with a
>less accurate one.

I'd add one qualificaion here: it may be that he wants to report this 
exception in particular, while still not "handling it". In which case 
I'd advocate something like:

  try:
    ... Popen stuff ...
  except subprocess.CalledProcessError as e:
    s = ....
    print(s, file=sys.stderr)
    raise

i.e. report some special message, then _reraise_ the original exception.

In this way he gets to keep the original exception and traceback for 
debugging, which still making whatever special message he wanted to 
make.

Cheers,
Cameron Simpson <cs at cskk.id.au>


More information about the Tutor mailing list