Francesco Romani has posted comments on this change. Change subject: virt: ExpiringCache: implement __nonzero__ ......................................................................
Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/39237/1/tests/vmUtilsTests.py File tests/vmUtilsTests.py: Line 103: clock = FakeClock(0.0) Line 104: cache = utils.ExpiringCache(ttl=1.0, clock=clock) Line 105: Line 106: ITEMS = 10 Line 107: for i in xrange(ITEMS): > where size is not the issue, and performance is not a big deal like in test yes. I have muscle memory wired to 'xrange', even though I agree I should really use 'range' like 90% of times. Will fix. Line 108: cache[i] = 'foobar-%d' % i Line 109: self.assertTrue(cache) Line 110: Line 111: clock.now = 1.1 https://gerrit.ovirt.org/#/c/39237/1/vdsm/virt/utils.py File vdsm/virt/utils.py: Line 90: now = self._clock() Line 91: with self._lock: Line 92: for key in [key Line 93: for key, (expiration, value) Line 94: in self._items.items() > the cache can get big, can it? maybe use iteritems() and worry about python The problem I was trying to solve and is that if we use items(), this will going to fail on py3 (dictionary changed while iterating). Hence, the need for a list of expired keys - and the list comprehension. Of course, it is foolish to use a list comprehension _AND_ items() at the same time :( Line 95: if expiration <= now]: Line 96: del self._items[key] Line 97: Line 98: return bool(self._items) -- To view, visit https://gerrit.ovirt.org/39237 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I15324dc8c43fbb4ef2acb260728f09bcb9a1c975 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[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
