Milan Zamazal has posted comments on this change. Change subject: rpc: Log important info from VM stats ......................................................................
Patch Set 19: (2 comments) https://gerrit.ovirt.org/#/c/58465/19/lib/vdsm/throttledlog.py File lib/vdsm/throttledlog.py: Line 39: self._timeout = timeout Line 40: self._counter = 0 Line 41: self._last_time = 0 Line 42: Line 43: def should_log(self): > This can also work, but this is the throttledlog module, not throttle mdoul I like letting the class doing its single job without messing with logging even when it's just a private utility class here. I think the code in this module is fine as it is, we just need a good name for this (and the following) method. Line 44: now = monotonic_time() Line 45: result = self._should_log(now) Line 46: self._counter = (self._counter + 1) % self._interval Line 47: self._last_time = now Line 70: :type interval: int Line 71: :param timeout: The number of seconds without log emitted after which Line 72: `log()` should always unthrottle the next message. Line 73: :type timeout: int Line 74: """ > Nicely documented, we should have more docstrings like this in vdsm. Thanks. :) Line 75: _periodic[name] = _Periodic(interval, timeout) Line 76: Line 77: Line 78: def log(name, level, message, *args): -- 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: 19 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]
