On 2015-10-27 17:24, Ganesh Pal wrote: > from myPopen import run > > def configure_network(): > """ > Prepare network for test > """ > try: > cmd = ("netadm enable -p ncp DefaultFixed") > out, err, ret = run(cmd, timeout=60) > if ret != "": > logging.error("Can't run %s got %s (%d)!" % (cmd, err, > ret)) return False > cmd = ("ipadm create-ip net3") > out, err, ret = run(cmd, timeout=60) > if ret != "": > logging.error("Can't run %s got %s (%d)!" % (cmd, err, > ret)) return False > cmd = ("ipadm create-addr -a 192.168.84.3/24 net3") > out, err, ret = run(cmd, timeout=60) > if ret != "": > logging.error("Can't run %s got %s (%d)!" % (cmd, err, > ret)) return False > cmd = (" route -p add default 192.168.84.1") > out, err, ret = run(cmd, timeout=60) > if ret != "": > logging.error("Can't run %s got %s (%d)!" % (cmd, err, > ret)) return False > except Exception, e: > logging.exception("Failed to run %s got %s" % (cmd, e)) > return False > logging.info("Configuring network .Done !!!") > return True > > > Q1.How to make this code look better (in terms of quality)
I'd be tempted 1) to put it in a loop 2) just test the "ret" string directly instead of comparing it to the empty string 3) to do your exception testing on each command rather than wrapping the entire loop for cmd in [ "netadm enable -p ncp DefaultFixed", "ipadm create-ip net3", "ipadm create-addr -a 192.168.84.3/24 net3", "route -p add default 192.168.84.1", ]: try: out, err, ret = run(cmd, timeout=60) if ret: logging.exception("Can't run %s got %s (d)!", cmd, err, ret) return False except Exception, e: logging.exception("Failed to run %s", cmd) return False logging.info("Configuring network done.") return True It reduces the redundant code and also brings all of the commands together in one place to see the expected steps. > Q2. Iam using the except clause, just to maintain the syntax, will > any exception be caught in this case. All regular exceptions will be caught. I think certain errors (rather than exceptions) may still behave properly. -tkc -- https://mail.python.org/mailman/listinfo/python-list