Nir Soffer has posted comments on this change.

Change subject: rpc: Log important info from VM stats
......................................................................


Patch Set 26:

(2 comments)

https://gerrit.ovirt.org/#/c/58465/26/tests/logutils_test.py
File tests/logutils_test.py:

Line 35:                    "u'bd0d066b-971e-42f8-8bc6-d647ab7e0e70': 'Powering 
up'}")
Line 36: 
Line 37:     def test_allvmstats(self):
Line 38:         data = AllVmStatsValue(self._STATS)
Line 39:         self.assertEqual(str(data), self._SIMPLIFIED)
This test may break on different python versions or architectures since dict 
iteration order is undefined.

A more robust test would be to keep _SIMPLIFIED as dict and parse the result:

    _SIMPLIFIED = {
        u'43f02a2d-e563-4f11-a7bc-9ee191cfeba1': 'Up',
        u'bd0d066b-971e-42f8-8bc6-d647ab7e0e70': 'Powering up',
    }
   
   result = str(data)
   self.assertEqual(eval(result), _SIMPLIFED)


https://gerrit.ovirt.org/#/c/58465/26/vdsm/API.py
File vdsm/API.py:

Line 1347:         hooks.before_get_all_vm_stats()
Line 1348:         statsList = self._cif.getAllVmStats()
Line 1349:         statsList = hooks.after_get_all_vm_stats(statsList)
Line 1350:         throttledlog.debug('getAllVmStats', "Current getAllVmStats: 
%s",
Line 1351:                            AllVmStatsValue(statsList))
Why not info? I thought you want to see this log when we move to info level?
Line 1352:         return {'status': doneCode, 'statsList': 
Suppressed(statsList)}
Line 1353: 
Line 1354:     def hostdevListByCaps(self, caps=None):
Line 1355:         devices = hostdev.list_by_caps(caps)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcbac615323b62fb9a27e5c0f5a4e98990076146
Gerrit-PatchSet: 26
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Milan Zamazal <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to