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