Path: csiph.com!news.swapon.de!fu-berlin.de!uni-berlin.de!not-for-mail
From: Ganesh Pal <ganesh1pal@gmail.com>
Newsgroups: comp.lang.python
Subject: Re: common mistakes in this simple program
Date: Mon, 29 Feb 2016 22:56:43 +0530
Lines: 105
Message-ID: <mailman.28.1456766811.20602.python-list@python.org>
References: <CACT3xuWUk02Or78XHTdkFLsy6CMVHEYKOyid3hFJxw8Wh6-g1A@mail.gmail.com> <CALwzid=3sN3ic=wrq8qn+0P+Zs8xGn1TJRLYL35B1Qk-vDeh5g@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
In-Reply-To: <CALwzid=3sN3ic=wrq8qn+0P+Zs8xGn1TJRLYL35B1Qk-vDeh5g@mail.gmail.com>
Precedence: list
Xref: csiph.com comp.lang.python:103739

On Mon, Feb 29, 2016 at 9:59 PM, Ian Kelly <ian.g.kelly@gmail.com> wrote:
> On Mon, Feb 29, 2016 at 8:18 AM, Ganesh Pal <ganesh1pal@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

>
>> def run_cmd_and_verify(cmd, timeout=1000):
>>     try:
>>         pdb.set_trace()
>>         out, err, ret = run(cmd, timeout=timeout)
>
> What is "run"? It's imported like a module above, but here you're
> using it like a function.

Sorry run is a function  which was imported from a library , the
function had to be  # from utility import run ,

>>         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

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 ?

#!/usr/bin/env python

"""
bugging code
"""
import logging
from utility import run

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)
    except Exception as e:
        logging.error("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):
                logging.info("Preparing cluster failed ...")
                return False
        except:
            pass
    logging.info("Preparing Cluster.....Done !!!")
    return True


def main():
    functions = [prep_host]
    for func in functions:
        try:
            func()
        except Exception as e:
            logging.info(e)
            return False
if __name__ == '__main__':
    main()


Regards,
Gpal