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