usage of try except for review.

[Steven D'Aprano]

On Mon, 29 Feb 2016 07:13 pm, Ganesh Pal wrote:


> def run_cmd_and_verify(cmd, timeout=1000):
>     try:
>         out, err, ret = run(cmd, timeout=timeout)
>         assert ret ==0,"ERROR (ret %d): " \
>                 " \nout: %s\nerr: %s\n" % (ret, out, err)

Do not use assert for error checking.

http://import-that.dreamwidth.org/676.html


Instead, you should write this:


        out, err, ret = run(cmd, timeout=timeout)
        if ret != 0:
            raise RuntimeError(
                  "ERROR (ret %d): \nout: %s\nerr: %s\n" % (ret, out, err))


>     except Exception as e:
>         logging.error("Failed to run %s got %s" % (cmd, e))
>         return False
>     return True
> 
> 
> def run_test():
>     """
>     Mount
>     """
>     pdb.set_trace()

Do not use the debugger as part of production code. The debugger is for
debugging. When you have debugged the section of code, remove the debugger
calls.


>     for cmd in ["mount /nfs_mount1", "mount /cifs_mount1"]:
>         try:
>             if not run_cmd_and_verify(cmd, timeout=3600):
>                 return False
>         except:
>            logging.error("Failure while running command %")
>     logging.info("Setup and Creation ....Done !!!")

Do not use bare except clauses like this unless you know what you are doing.

https://realpython.com/blog/python/the-most-diabolical-python-antipattern/



>     cmd = "run_scan"
>     out, err, ret = run(cmd)
>     for cmd in ["create_data.py -nfs ",
>                 "validate.py -30 "]:
>         try:
>             if not run_cmd_and_verify(cmd, timeout=3600):
>                return False
>         except:
>             logging.error("")
>             return False
>     logging.info("Mount IS START.....Done !!!")
> 
> def main():
>     if not run_test():
>         sys.exit("Exiting Main")
> 
> 
> if __name__ == '__main__':
>     main()


I would re-write this script as something like this:



# Untested

def run_cmd(cmd, timeout=1000):
    out, err, ret = run(cmd, timeout=timeout)
    if ret != 0:
        raise RuntimeError(
              "ERROR (ret %d): \nout: %s\nerr: %s\n" % (ret, out, err))


def run_test():
    for cmd in ["mount /nfs_mount1", "mount /cifs_mount1"]:
        run_cmd(cmd, timeout=3600)
    logging.info("Setup and Creation ....Done !!!")
    cmd = "run_scan"
    out, err, ret = run(cmd)
    # Do you care about the result of the scan? Then you should log it.
    for cmd in ["create_data.py -nfs ", "validate.py -30 "]:
        run_cmd(cmd, timeout=3600)
    logging.info("Mount IS START.....Done !!!")
 

if __name__ == '__main__':
    try:
        run_test()
    except Exception as err:
        logging.error("run_test failed", exc_info=True)
        sys.exit(1)



> Question 1:
> 
> 
> 
> 1. Have I used try and expect block correctly  ? , In my case I have
> the except block that's  is not needed it just gives an  message I
> have still included for the sake of try block

If the except block is not needed, then you should not use try.


> 2.  If a failure’s are encountered  the error by assert condition the
> errors are now displayed on the screen , how do I redirect it to log
> file using logging error

Don't use assert like that.



> 3. my function def has 1000 but Iam using 3600 in the calling fnx etc
> , Time out value are overwritten ?

I don't understand the question.



-- 
Steven