Change in vdsm[master]: virt: add ExpiringCache
automat...@ovirt.org has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 11: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
automat...@ovirt.org has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 10: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
automat...@ovirt.org has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 9: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
automat...@ovirt.org has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 12: * Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
automat...@ovirt.org has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 8: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
automat...@ovirt.org has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 7: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 12: Build Failed http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el6-x86_64_merged/629/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-fc21-x86_64_merged/228/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/4612/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc21-x86_64_merged/605/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-el6-x86_64_merged/232/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el7-x86_64_merged/630/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-el7-x86_64_merged/232/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master-libgfapi_create-rpms-fc20-x86_64_merged/218/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc20-x86_64_merged/625/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/6451/ : FAILURE -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Dan Kenigsberg has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 11: Code-Review+2 -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Dan Kenigsberg has submitted this change and it was merged. Change subject: virt: add ExpiringCache .. virt: add ExpiringCache ExpiringCache is a simple cache with expiring time attached to keys. This class implements parts of the dict() interface, but do not aims to be a full replacement for dict()s. ExpiringCache is thread safe. Change-Id: I51e38cea6a23b7abe2375780e6ece0ff90831b6d Signed-off-by: Francesco Romani Reviewed-on: http://gerrit.ovirt.org/36716 Reviewed-by: Nir Soffer Reviewed-by: Dan Kenigsberg --- M tests/Makefile.am A tests/vmUtilsTests.py M vdsm/virt/utils.py 3 files changed, 161 insertions(+), 0 deletions(-) Approvals: Nir Soffer: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified -- To view, visit http://gerrit.ovirt.org/36716 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I51e38cea6a23b7abe2375780e6ece0ff90831b6d Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Nir Soffer has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 11: Code-Review+1 -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 11: Build Failed http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/2359/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/15561/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14759/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15730/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created_staging/537/ : FAILURE -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Francesco Romani has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 11: Verified+1 fixed docstring. Verified using the tests. -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Francesco Romani has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 10: (1 comment) http://gerrit.ovirt.org/#/c/36716/10/vdsm/virt/utils.py File vdsm/virt/utils.py: Line 77: Line 78: def __getitem__(self, key): Line 79: """ Line 80: Raises KeyError if a key was not found. Line 81: Raises ItemExpired if a a key expired and was cleaned up. > Still with double a? Damn, I missed the double 'a'. Also, will rephrase as you suggested. Line 82: ItemExpired is a subclass of KeyError. Line 83: """ Line 84: return self._get_live(key) Line 85: -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer 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
Change in vdsm[master]: virt: add ExpiringCache
Nir Soffer has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 10: (1 comment) http://gerrit.ovirt.org/#/c/36716/10/vdsm/virt/utils.py File vdsm/virt/utils.py: Line 77: Line 78: def __getitem__(self, key): Line 79: """ Line 80: Raises KeyError if a key was not found. Line 81: Raises ItemExpired if a a key expired and was cleaned up. Still with double a? I think this can be nicer as a single paragraph: If no item is stored for key, raises KeyError. If an item expired, raises ItemExpired (a subclass of KeyError). Line 82: ItemExpired is a subclass of KeyError. Line 83: """ Line 84: return self._get_live(key) Line 85: -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer 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
Change in vdsm[master]: virt: add ExpiringCache
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 10: Build Failed http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/2346/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/15539/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14737/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15709/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created_staging/516/ : FAILURE -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Francesco Romani has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 10: version 10 is a rebase, which was needed because a patch later in the series is affected by recent merge of a patch which cleaned up a little getNicStats -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Dan Kenigsberg has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 9: Code-Review+1 -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Francesco Romani has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 9: (1 comment) http://gerrit.ovirt.org/#/c/36716/9/vdsm/virt/utils.py File vdsm/virt/utils.py: Line 77: Line 78: def __getitem__(self, key): Line 79: """ Line 80: Raises KeyError if a key was not found. Line 81: Raises ItemExpired if a a key expired and was cleaned up. typo: "a a" Line 82: ItemExpired is a subclass of KeyError. Line 83: """ Line 84: return self._get_live(key) Line 85: -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer 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
Change in vdsm[master]: virt: add ExpiringCache
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 9: Build Failed http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/2326/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/15481/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14678/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15650/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created_staging/456/ : FAILURE -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Francesco Romani has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 9: this version addresses the comments from Nir (thanks!) -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Francesco Romani 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: agreed and done 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. Removed. 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. Done 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 Not at present moment, so removed. 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 n Done 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 t Agreed and done. 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? get_live is probably a little nicer. Changed. 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__ Done 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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer 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
Change in vdsm[master]: virt: add ExpiringCache
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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches m
Change in vdsm[master]: virt: add ExpiringCache
oVirt Jenkins CI Server has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 8: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15561/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/15393/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/2310/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14588/ : FAILURE -- 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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Francesco Romani has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 8: this version implements suggestions and remarks from Nir. - minor code and docstrings cleanups - improved and more comprehensive tests. -- 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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add ExpiringCache
Francesco Romani has posted comments on this change.
Change subject: virt: add ExpiringCache
..
Patch Set 7:
(11 comments)
http://gerrit.ovirt.org/#/c/36716/7/tests/vmUtilsTests.py
File tests/vmUtilsTests.py:
Line 41:
Line 42: def test_get_equals_getitem(self):
Line 43: self.cache['the answer'] = 42
Line 44: self.assertEqual(42, self.cache['the answer'])
Line 45: self.assertEqual(42, self.cache.get('the answer'))
> This seems to duplicate the previous tests, and does not add any value.
Removed.
Line 46:
Line 47: def test_setitem_get_fallback(self):
Line 48: self.cache['the answer'] = 42
Line 49: value = self.cache.get('a different answer', )
Line 44: self.assertEqual(42, self.cache['the answer'])
Line 45: self.assertEqual(42, self.cache.get('the answer'))
Line 46:
Line 47: def test_setitem_get_fallback(self):
Line 48: self.cache['the answer'] = 42
> You don't need to set a value for testing default.
Done
Line 49: value = self.cache.get('a different answer', )
Line 50: self.assertEqual(value, )
Line 51:
Line 52: def test_getitem_missing(self):
Line 45: self.assertEqual(42, self.cache.get('the answer'))
Line 46:
Line 47: def test_setitem_get_fallback(self):
Line 48: self.cache['the answer'] = 42
Line 49: value = self.cache.get('a different answer', )
> Using "default" instead of will be more clear.
Done
Line 50: self.assertEqual(value, )
Line 51:
Line 52: def test_getitem_missing(self):
Line 53: self.cache['foo'] = 'bar'
Line 46:
Line 47: def test_setitem_get_fallback(self):
Line 48: self.cache['the answer'] = 42
Line 49: value = self.cache.get('a different answer', )
Line 50: self.assertEqual(value, )
> Please add a test that get existing value and does not use the default:
Done
Line 51:
Line 52: def test_getitem_missing(self):
Line 53: self.cache['foo'] = 'bar'
Line 54: self.assertRaises(KeyError,
Line 49: value = self.cache.get('a different answer', )
Line 50: self.assertEqual(value, )
Line 51:
Line 52: def test_getitem_missing(self):
Line 53: self.cache['foo'] = 'bar'
> You don't need to add a value for testing missing value.
Done
Line 54: self.assertRaises(KeyError,
Line 55: lambda key: self.cache[key],
Line 56: 'FIZZBUZZ')
Line 57:
Line 57:
Line 58: def test_delitem(self):
Line 59: self.cache['the answer'] = 42
Line 60: del self.cache['the answer']
Line 61: self.assertEquals(self.cache.get('the answer'), None)
> This should be another tests, checking the return value from get without a
Agreed and done.
Line 62:
Line 63: def test_delitem_missing(self):
Line 64: def _del(key):
Line 65: del self.cache[key]
Line 76:
Line 77: for i in xrange(ITEMS):
Line 78: self.cache.get(i) is None
Line 79:
Line 80: def test_key_expiration(self):
> This is the only test that needs the counter. I would create the cache with
Done
Line 81: ITEMS = 10
Line 82: for i in xrange(ITEMS):
Line 83: self.cache[i] = 'foobar-%d' % i
Line 84:
Line 87: for _ in xrange(100):
Line 88: self._counter.next()
Line 89:
Line 90: for i in xrange(ITEMS):
Line 91: self.assertEqual(self.cache.get(i), None)
> This is not a good test since you do not check the edge cases:
Good point. Changed.
http://gerrit.ovirt.org/#/c/36716/7/vdsm/virt/utils.py
File vdsm/virt/utils.py:
Line 39: return all(k in drive for k in required)
Line 40:
Line 41:
Line 42: class ItemExpired(KeyError):
Line 43: pass
> Having a docstring is more useful then pass, although in this case I don't
Agreed. Can't find something meaningful to use as docstring, so I used 'pass'
Line 44:
Line 45:
Line 46: class ExpiringCache(object):
Line 47: """
Line 53: the time of their inserion in the ExpiringCache
Line 54: clock: time.time() or monotonic_time()-like callable.
Line 55:
Line 56: Expired keys are transparently removed on the first attempt
Line 57: to access them, using get or __getitem__
> It would be nice to document that we raise ItemExpired error, in case some
Done
Line 58: """
Line 59: def __init__(self, ttl, clock=monotonic_time):
Line 60: self._ttl = ttl
Line 61: self._clock = clock
Line 86:
Line 87: # private
Line 88:
Line 89: def _get(self, key):
Line 90: # Automatically cleanups expired keys.
> Use docstring, or rename to something that explain the behavior like _get_a
Done both. Renamed _get_unexpired because I dislike names with conjunctions in
them (and, or). Extended the comment and changed to docstring.
Line 91: now =
Change in vdsm[master]: virt: add ExpiringCache
Nir Soffer has posted comments on this change.
Change subject: virt: add ExpiringCache
..
Patch Set 7:
(11 comments)
Nice!
To ensure that we are not wasting time on utility which may not be needed - can
you point us to the patch using it?
http://gerrit.ovirt.org/#/c/36716/7/tests/vmUtilsTests.py
File tests/vmUtilsTests.py:
Line 41:
Line 42: def test_get_equals_getitem(self):
Line 43: self.cache['the answer'] = 42
Line 44: self.assertEqual(42, self.cache['the answer'])
Line 45: self.assertEqual(42, self.cache.get('the answer'))
This seems to duplicate the previous tests, and does not add any value.
Line 46:
Line 47: def test_setitem_get_fallback(self):
Line 48: self.cache['the answer'] = 42
Line 49: value = self.cache.get('a different answer', )
Line 44: self.assertEqual(42, self.cache['the answer'])
Line 45: self.assertEqual(42, self.cache.get('the answer'))
Line 46:
Line 47: def test_setitem_get_fallback(self):
Line 48: self.cache['the answer'] = 42
You don't need to set a value for testing default.
Line 49: value = self.cache.get('a different answer', )
Line 50: self.assertEqual(value, )
Line 51:
Line 52: def test_getitem_missing(self):
Line 45: self.assertEqual(42, self.cache.get('the answer'))
Line 46:
Line 47: def test_setitem_get_fallback(self):
Line 48: self.cache['the answer'] = 42
Line 49: value = self.cache.get('a different answer', )
Using "default" instead of will be more clear.
Line 50: self.assertEqual(value, )
Line 51:
Line 52: def test_getitem_missing(self):
Line 53: self.cache['foo'] = 'bar'
Line 46:
Line 47: def test_setitem_get_fallback(self):
Line 48: self.cache['the answer'] = 42
Line 49: value = self.cache.get('a different answer', )
Line 50: self.assertEqual(value, )
Please add a test that get existing value and does not use the default:
self.cache['the answer'] = 42
self.assertEqual(42, self.cache.get('the answer', 'default'))
Line 51:
Line 52: def test_getitem_missing(self):
Line 53: self.cache['foo'] = 'bar'
Line 54: self.assertRaises(KeyError,
Line 49: value = self.cache.get('a different answer', )
Line 50: self.assertEqual(value, )
Line 51:
Line 52: def test_getitem_missing(self):
Line 53: self.cache['foo'] = 'bar'
You don't need to add a value for testing missing value.
Line 54: self.assertRaises(KeyError,
Line 55: lambda key: self.cache[key],
Line 56: 'FIZZBUZZ')
Line 57:
Line 57:
Line 58: def test_delitem(self):
Line 59: self.cache['the answer'] = 42
Line 60: del self.cache['the answer']
Line 61: self.assertEquals(self.cache.get('the answer'), None)
This should be another tests, checking the return value from get without a
default.
Line 62:
Line 63: def test_delitem_missing(self):
Line 64: def _del(key):
Line 65: del self.cache[key]
Line 76:
Line 77: for i in xrange(ITEMS):
Line 78: self.cache.get(i) is None
Line 79:
Line 80: def test_key_expiration(self):
This is the only test that needs the counter. I would create the cache with
default clock for all the tests except expiring tests, and move the expiring
tests to new test case class.
Line 81: ITEMS = 10
Line 82: for i in xrange(ITEMS):
Line 83: self.cache[i] = 'foobar-%d' % i
Line 84:
Line 87: for _ in xrange(100):
Line 88: self._counter.next()
Line 89:
Line 90: for i in xrange(ITEMS):
Line 91: self.assertEqual(self.cache.get(i), None)
This is not a good test since you do not check the edge cases:
- time < expire time - 0.01
- time == expire time
- time > expire time
I don't think the counter is a good way to test this, better have a fake clock
which you can set before the test:
class FakeClock(object):
def __init__(self, now):
self.now = now
def __call__(self):
return self.now
class ExpireTests(TestCaseBase):
def test_expired(self):
clock = FakeClock(0.0)
cache = ExpiringCahce(ttl=1.0, clock=clock)
cache['the answer'] = 42
clock.now = 0.99
self.assertEqual(42, cache['the answer'])
clock.now = 1.0
self.assertEqual(42, cache['the answer']) # Should this fail?
clock.now = 1.01
self.assertEqual(None, cache.get('the answer'))
http://gerrit.ovirt.org/#/c/36716/7/vdsm/virt/utils.py
File vdsm/virt/utils.py:
Line 39: return all(k in drive for k in required)
Line 40:
Line 41:
Line 42: class ItemExpired(KeyError):
Line 43: pass
Having a docstring is more useful then pass
Change in vdsm[master]: virt: add ExpiringCache
Francesco Romani has posted comments on this change. Change subject: virt: add ExpiringCache .. Patch Set 7: Address Nir's comments. -- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
