Change in vdsm[master]: osinfo: avoid excepting *

fromani at redhat.com fromani at redhat.com
Wed Mar 9 12:55:02 UTC 2016


Francesco Romani has posted comments on this change.

Change subject: osinfo: avoid excepting *
......................................................................


Patch Set 1: Code-Review-1

(6 comments)

I like this patch, but -1 for visibility of the inline comments

https://gerrit.ovirt.org/#/c/54545/1/lib/vdsm/osinfo.py
File lib/vdsm/osinfo.py:

Line 156:     def kernelDict():
Line 157:         try:
Line 158:             ret = os.uname()
Line 159:             ver, rel = ret[2].split('-', 1)
Line 160:         except Exception:
let's try to be more specific. Maybe ValueError here?
Line 161:             logging.error('kernel release not found', exc_info=True)
Line 162:             ver, rel = '0', '0'
Line 163:         try:
Line 164:             t = ret[3].split()[2:]


Line 163:         try:
Line 164:             t = ret[3].split()[2:]
Line 165:             del t[4]  # Delete timezone
Line 166:             t = time.mktime(time.strptime(' '.join(t)))
Line 167:         except Exception:
same
Line 168:             logging.error('kernel build time not found', exc_info=True)
Line 169:             t = '0'
Line 170:         return dict(version=ver, release=rel, buildtime=t)
Line 171: 


Line 193:             for pkg, names in KEY_PACKAGES.iteritems():
Line 194:                 try:
Line 195:                     mi = itertools.chain(*[ts.dbMatch('name', name)
Line 196:                                            for name in names]).next()
Line 197:                 except StopIteration:
uh, this is ugly :\
Line 198:                     logging.debug("rpm package %s not found",
Line 199:                                   KEY_PACKAGES[pkg])
Line 200:                 else:
Line 201:                     pkgs[pkg] = {


Line 202:                         'version': mi['version'],
Line 203:                         'release': mi['release'],
Line 204:                         'buildtime': mi['buildtime'],
Line 205:                     }
Line 206:         except Exception:
let's keep it this way. Maybe let's narrow down the try/except block in a future patch.
Line 207:             logging.error('', exc_info=True)
Line 208: 
Line 209:     elif _release() == OSName.DEBIAN and python_apt:
Line 210:         KEY_PACKAGES = {


Line 203:                         'release': mi['release'],
Line 204:                         'buildtime': mi['buildtime'],
Line 205:                     }
Line 206:         except Exception:
Line 207:             logging.error('', exc_info=True)
let's use logging.exception() in a future patch.
Line 208: 
Line 209:     elif _release() == OSName.DEBIAN and python_apt:
Line 210:         KEY_PACKAGES = {
Line 211:             'glusterfs-cli': 'glusterfs-cli',


Line 228:                 deb_pkg = KEY_PACKAGES[pkg]
Line 229:                 ver = cache[deb_pkg].installed.version
Line 230:                 # Debian just offers a version
Line 231:                 pkgs[pkg] = dict(version=ver, release="", buildtime="")
Line 232:             except Exception:
maybe narrow down?
Line 233:                 logging.error('', exc_info=True)
Line 234: 


-- 
To view, visit https://gerrit.ovirt.org/54545
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ice9171e64fb6b3879a33f5d9566e8aa2546c424f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list