common mistakes in this simple program

[Ian Kelly]

On Mon, Feb 29, 2016 at 10:26 AM, Ganesh Pal <ganesh1pal at gmail.com> wrote:
> On Mon, Feb 29, 2016 at 9:59 PM, Ian Kelly <ian.g.kelly at gmail.com> wrote:
>> On Mon, Feb 29, 2016 at 8:18 AM, Ganesh Pal <ganesh1pal at gmail.com> wrote:
>>> Iam on python 2.6
>
>>> 1. usage of try- expect
>>
>> try-except in every single function is a code smell. You should only
>> be using it where you're actually going to handle the exception. If
>> you catch an exception just to log it, you generally should also
>> reraise it so that something further up the call chain has the
>> opportunity to handle it.
>
> How do we reraise the exception in python ,  I have used raise not
> sure how to reraise the exception

raise with no arguments will reraise the exception currently being handled.

except Exception:
    logging.error("something went wrong")
    raise

>>>         assert ret ==0,"ERROR (ret %d): " \
>>>                 " \nout: %s\nerr: %s\n" % (ret, out, err)
>>>     except Exception as e:
>>>         print("Failed to run %s got %s" % (cmd, e))
>>>         return False
>>>     return True
>>>
>>> def prep_host():
>>>     """
>>>     Prepare clustering
>>>     """
>>>     for cmd in ["ls -al",
>>>                 "touch /tmp/file1",
>>>                 "mkdir /tmp/dir1"]:
>>>         try:
>>>             if not run_cmd_and_verify(cmd, timeout=3600):
>>>                 return False
>>>         except:
>>
>> What exceptions are you expecting this to catch? run_cmd_and_verify
>> already catches any expected exceptions that it raises.
>
> This is a wrapper for Popen it runs the command and returns stdout
> ,stderror and returncode .The only exception it can return is a time
> out exception

But that exception is already caught by the run_cmd_and_verify
function, so what exception are you expecting to be caught *here*?

> Here is the modified buggy code ,  Can I have Try and except with Pass
> , how do I modify the  try and expect in the pre-host ?

You should virtually never just pass in an exception handler. Either
handle the exception, or log it and reraise it. If you're going to do
neither of those things, then don't use a try-except at all.