Francesco Romani has posted comments on this change. Change subject: introduce connectivity log ......................................................................
Patch Set 1: (4 comments) OK with the concept, some codestyle questions. http://gerrit.ovirt.org/#/c/29472/1/tests/samplingTests.py File tests/samplingTests.py: Line 101: def testBootTimeExtra(self): Line 102: with MonkeyPatchScope([(sampling, '_PROC_STAT_PATH', Line 103: self._extra_path)]): Line 104: self.assertEquals(sampling.getBootTime(), 1395249141) Line 105: one newline is missing Line 106: class InterfaceSampleTests(TestCaseBase): Line 107: def testDiff(self): Line 108: lo = ipwrapper.getLink('lo') Line 109: s0 = sampling.InterfaceSample(lo) http://gerrit.ovirt.org/#/c/29472/1/vdsm/logger.conf.in File vdsm/logger.conf.in: Line 61: args: [] Line 62: formatter: none Line 63: Line 64: [formatter_simple] Line 65: format: %(asctime)s:%(levelname)s:%(message)s Maybe this deserves a separate patch Line 66: Line 67: [formatter_none] Line 68: format: %(message)s Line 69: http://gerrit.ovirt.org/#/c/29472/1/vdsm/virt/sampling.py File vdsm/virt/sampling.py: Line 137: and missing in 'other'.""" Line 138: Line 139: return self._log_attrs( Line 140: attr for attr in self._LOGGED_ATTRS Line 141: if getattr(self, attr) != getattr(other, attr)) I'm admittedly low on caffeine right now, but I'm not sure all this getattr() dance is better than explicitely referencing the attributes, since the one to be logged are just two. Do you plan to extend the logging in the near future? Line 142: Line 143: Line 144: class TotalCpuSample: Line 145: """ Line 513: some tabs sneaked in -- To view, visit http://gerrit.ovirt.org/29472 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I86dc41223babe43194add7ce778c6567b2bc5823 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
