Change in vdsm[master]: Introducing getAvailableUpdates API
danken at redhat.com
danken at redhat.com
Mon Mar 30 12:59:11 UTC 2015
Dan Kenigsberg has posted comments on this change.
Change subject: Introducing getAvailableUpdates API
......................................................................
Patch Set 9: Code-Review-1
(3 comments)
https://gerrit.ovirt.org/#/c/38559/9/vdsm/API.py
File vdsm/API.py:
Line 1311: with open(constants.P_VDSM_LIB + 'updates.list', 'r') as f:
Line 1312: updates = json.loads(f.read())
Line 1313:
Line 1314: for pkgInfo in updates:
Line 1315: if packages is None or pkgInfo['name'] in packages:
how long is the expected list of packages? it may make sense to translate packages into a frozenset for quicker lookup.
Line 1316: info['updates'].append(pkgInfo)
Line 1317:
Line 1318: return {'status': doneCode, 'info': info}
Line 1319: except:
Line 1314: for pkgInfo in updates:
Line 1315: if packages is None or pkgInfo['name'] in packages:
Line 1316: info['updates'].append(pkgInfo)
Line 1317:
Line 1318: return {'status': doneCode, 'info': info}
I think it would be good to report how old is the cache. If it's older than one hour, the Engine can report an alert.
Line 1319: except:
Line 1320: self.log.error("failed to read update info", exc_info=True)
Line 1321: return errCode['updateInfoErr']
Line 1322:
Line 1317:
Line 1318: return {'status': doneCode, 'info': info}
Line 1319: except:
Line 1320: self.log.error("failed to read update info", exc_info=True)
Line 1321: return errCode['updateInfoErr']
is there a real need to translate all execptions to this error? is it really better than the default translation into "unexpected error"?
Line 1322:
Line 1323: def getAllVmStats(self):
Line 1324: """
Line 1325: Get statistics of all running VMs.
--
To view, visit https://gerrit.ovirt.org/38559
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d47d5395519fff4378ab7dd88b30c4766467427
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag at redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list