Change in vdsm[master]: sampling: extract SampleWindow class
fromani at redhat.com
fromani at redhat.com
Mon Oct 27 14:19:00 UTC 2014
Francesco Romani has posted comments on this change.
Change subject: sampling: extract SampleWindow class
......................................................................
Patch Set 4:
(4 comments)
http://gerrit.ovirt.org/#/c/33783/4/vdsm/virt/sampling.py
File vdsm/virt/sampling.py:
Line 305:
Line 306: class SampleWindow(object):
Line 307: """
Line 308: Sliding window of samples. Automatically record
Line 309: the timestamp of the given samples.
> Maybe something like: "Keep sliding window of samples"
Clear and simple. Done.
Line 310: """
Line 311: def __init__(self, size=0, timefn=time.time):
Line 312: self._samples = deque(maxlen=size)
Line 313: self._timefn = timefn
Line 307: """
Line 308: Sliding window of samples. Automatically record
Line 309: the timestamp of the given samples.
Line 310: """
Line 311: def __init__(self, size=0, timefn=time.time):
> Is 0 a useful default size? Seems like an invalid value for sliding window.
This is a (bad) inheritance from advancedStatsFunction (I don't know yet why it is like this, most likely historical reason lost in time).
So, collections.deque already raises ValueError if fed with maxlen<0. We all agree size >= 2 makes fully sense, what is left out is the strange corner cases of size in (0, 1).
BTW, collections.deque accepts these, but I don't think this class should.
For the sake of making this patch minimal, because AdvancedStatsFunction depends on size=1 being supported, I'll make and enforce size >= 1. In followup patches we'll discuss the most appropiate value.
Line 312: self._samples = deque(maxlen=size)
Line 313: self._timefn = timefn
Line 314:
Line 315: def append(self, value):
Line 314:
Line 315: def append(self, value):
Line 316: """
Line 317: Adds a new sample, and automatically record
Line 318: the timestamp of the new value.
> Maybe: "Record the current time and append new sample, removing the oldest
Done
Line 319: """
Line 320: timestamp = self._timefn()
Line 321: self._samples.append((timestamp, value))
Line 322:
Line 322:
Line 323: def stats(self):
Line 324: """
Line 325: Return a tuple in the format: (first, last, difftime), containing
Line 326: the first and the last return value in the defined 'window' and the
> BTW, I prefer the imperative tense, as that's what pep8 suggests.
Changed as Nir suggested, but using (everywhere) imperative tense as Dan prefers.
Line 327: time difference.
Line 328: """
Line 329: if len(self._samples) < 2:
Line 330: return None, None, None
--
To view, visit http://gerrit.ovirt.org/33783
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I48fe9e7f8ed3f5ec677d8f91a82cb70d0bdbb2e7
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer 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