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 <mpoled...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches