Nir Soffer has posted comments on this change. Change subject: virt: add ExpiringCache ......................................................................
Patch Set 8: (8 comments) http://gerrit.ovirt.org/#/c/36716/8/vdsm/virt/utils.py File vdsm/virt/utils.py: Line 49: is attached to each key. Thread safe. Line 50: Line 51: Parameters: Line 52: ttl: validity of the keys in `clock' time units since Line 53: the time of their inserion in the ExpiringCache. This can be simplified and be more clear: ttl: items will expire ttl seconds after they were inserted. Line 54: clock: time.time() or monotonic_time()-like callable. Line 55: Line 56: NOTE: The time interval of key validity is [T, T+ttl) Line 57: where `T' is the clock() time of insertion. Line 53: the time of their inserion in the ExpiringCache. Line 54: clock: time.time() or monotonic_time()-like callable. Line 55: Line 56: NOTE: The time interval of key validity is [T, T+ttl) Line 57: where `T' is the clock() time of insertion. This is not clear. I think the description of ttl is good enough. Line 58: Line 59: Expired keys are transparently removed on the first attempt Line 60: to access them, using get or __getitem__. In that case, Line 61: ItemExpired is raised; in case of a missing key, KeyError Line 55: Line 56: NOTE: The time interval of key validity is [T, T+ttl) Line 57: where `T' is the clock() time of insertion. Line 58: Line 59: Expired keys are transparently removed on the first attempt transparently can be removed. Line 60: to access them, using get or __getitem__. In that case, Line 61: ItemExpired is raised; in case of a missing key, KeyError Line 62: is raised as for regular dict()s. Line 63: """ Line 56: NOTE: The time interval of key validity is [T, T+ttl) Line 57: where `T' is the clock() time of insertion. Line 58: Line 59: Expired keys are transparently removed on the first attempt Line 60: to access them, using get or __getitem__. In that case, "using get or __getitem__" does not add anything. There is no other way to access items, right? Line 61: ItemExpired is raised; in case of a missing key, KeyError Line 62: is raised as for regular dict()s. Line 63: """ Line 64: def __init__(self, ttl, clock=monotonic_time): Line 57: where `T' is the clock() time of insertion. Line 58: Line 59: Expired keys are transparently removed on the first attempt Line 60: to access them, using get or __getitem__. In that case, Line 61: ItemExpired is raised; in case of a missing key, KeyError The discussion about ItemExpired is relevant only to __getitem__, get wil never raise. Moving this discussion to __getitem__ docstring will make it more clear. Line 62: is raised as for regular dict()s. Line 63: """ Line 64: def __init__(self, ttl, clock=monotonic_time): Line 65: self._ttl = ttl Line 58: Line 59: Expired keys are transparently removed on the first attempt Line 60: to access them, using get or __getitem__. In that case, Line 61: ItemExpired is raised; in case of a missing key, KeyError Line 62: is raised as for regular dict()s. People reading this may miss the fact that ItemExpired is a KeyError. The text should make it clear that if item is missing or expired, KeyError is raised. If the user want to detect that item was expired, it should catch ItemExpired. Line 63: """ Line 64: def __init__(self, ttl, clock=monotonic_time): Line 65: self._ttl = ttl Line 66: self._clock = clock Line 90: del self._items[key] Line 91: Line 92: # private Line 93: Line 94: def _get_unexpired(self, key): This name is strange. Maybe _get_live? Line 95: """ Line 96: Automatically cleanups expired keys. Line 97: Raises KeyError if a key was not found. Line 98: Raises ItemExpired if an expired key is cleaned up. Line 94: def _get_unexpired(self, key): Line 95: """ Line 96: Automatically cleanups expired keys. Line 97: Raises KeyError if a key was not found. Line 98: Raises ItemExpired if an expired key is cleaned up. This documentation belongs to __getitem__ Line 99: """ Line 100: now = self._clock() Line 101: with self._lock: Line 102: expiration, value = self._items[key] -- To view, visit http://gerrit.ovirt.org/36716 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I51e38cea6a23b7abe2375780e6ece0ff90831b6d Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches