Hello Piotr Kliczewski, Nir Soffer, Dan Kenigsberg,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/40029
to review the following change.
Change subject: utils: Add memoized invalidation ......................................................................
utils: Add memoized invalidation
We learned in the hard way that memoizing and monkey-patching do work nicely together. However, disabling memoizing during the tests means that we do not test the same code in the application.
This patch adds invalidate() method to memoized functions, so tests can clean up properly after they modify memozied data.
This can also be used by application code to invalidate memoized functions in certain conditions.
Change-Id: I3053b8ecc567f7c3154f7473dfd6b587823313ae Signed-off-by: Nir Soffer nsoffer@redhat.com Reviewed-on: http://gerrit.ovirt.org/37788 Reviewed-by: Francesco Romani fromani@redhat.com Reviewed-by: Piotr Kliczewski piotr.kliczewski@gmail.com Reviewed-by: Dan Kenigsberg danken@redhat.com --- M lib/vdsm/utils.py M tests/utilsTests.py 2 files changed, 67 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/40029/1
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index 4619b74..944cf4d 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -971,9 +971,14 @@ self.cache[args] = value return value
+ def invalidate(self): + self.cache.clear() + def __get__(self, obj, objtype): """Support instance methods.""" - return functools.partial(self.__call__, obj) + wrapper = functools.partial(self.__call__, obj) + wrapper.invalidate = self.cache.clear + return wrapper
def validateMinimalKeySet(dictionary, reqParams): diff --git a/tests/utilsTests.py b/tests/utilsTests.py index 5b3614d..2a0b8ef 100644 --- a/tests/utilsTests.py +++ b/tests/utilsTests.py @@ -18,6 +18,7 @@ # Refer to the README and COPYING files for full details of the license #
+import collections import os.path import contextlib import errno @@ -662,3 +663,63 @@ self.assertTrue( hack < base/2, "picklecopy [%f] not faster than deepcopy [%f]" % (hack, base)) + + +@expandPermutations +class MemoizedTests(TestCaseBase): + + def setUp(self): + self.values = {} + self.accessed = collections.defaultdict(int) + + @permutations([[()], [("a",)], [("a", "b")]]) + def test_memoized_method(self, args): + self.values[args] = 42 + self.assertEqual(self.accessed[args], 0) + self.assertEqual(self.memoized_method(*args), 42) + self.assertEqual(self.accessed[args], 1) + self.assertEqual(self.memoized_method(*args), 42) + self.assertEqual(self.accessed[args], 1) + + @permutations([[()], [("a",)], [("a", "b")]]) + def test_memoized_function(self, args): + self.values[args] = 42 + self.assertEqual(self.accessed[args], 0) + self.assertEqual(memoized_function(self, *args), 42) + self.assertEqual(self.accessed[args], 1) + self.assertEqual(memoized_function(self, *args), 42) + self.assertEqual(self.accessed[args], 1) + + def test_key_error(self): + self.assertRaises(KeyError, self.memoized_method) + self.assertRaises(KeyError, self.memoized_method, "a") + self.assertRaises(KeyError, self.memoized_method, "a", "b") + + def test_invalidate_method(self): + args = ("a",) + self.values[args] = 42 + self.assertEqual(self.memoized_method(*args), 42) + self.memoized_method.invalidate() + self.assertEqual(self.memoized_method(*args), 42) + self.assertEqual(self.accessed[args], 2) + + def test_invalidate_function(self): + args = ("a",) + self.values[args] = 42 + self.assertEqual(memoized_function(self, *args), 42) + memoized_function.invalidate() + self.assertEqual(memoized_function(self, *args), 42) + self.assertEqual(self.accessed[args], 2) + + @utils.memoized + def memoized_method(self, *args): + return self.get(args) + + def get(self, key): + self.accessed[key] += 1 + return self.values[key] + + +@utils.memoized +def memoized_function(test, *args): + return test.get(args)
automation@ovirt.org has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 1: Verified-1
* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * Check merged to previous::OK, change not open on any previous branch
Piotr Kliczewski has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 1: Code-Review+1
automation@ovirt.org has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * Check merged to previous::OK, change not open on any previous branch
automation@ovirt.org has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 3:
* Update tracker::#1267851::OK * Check Bug-Url::OK * Check Public Bug::#1267851::OK, public bug * Check Product::#1267851::OK, Correct classification oVirt * Check TR::#1267851::ERROR, wrong target release for stable branch, --- should match ^3.[54321].* * Check merged to previous::OK, change not open on any previous branch
Francesco Romani has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 3: Verified+1
tests passes, and this code will be used only in tests: V+1
Nir Soffer has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 3: Code-Review+1
Piotr Kliczewski has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 3: Code-Review+1
Francesco Romani has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 3: Continuous-Integration+1
run CI checks manually
automation@ovirt.org has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 4:
* Update tracker::#1267851::OK * Check Bug-Url::OK * Check Public Bug::#1267851::OK, public bug * Check Product::#1267851::OK, Correct classification oVirt * Check TR::#1267851::ERROR, wrong target release for stable branch, --- should match ^3.[54321].* * Check merged to previous::OK, change not open on any previous branch
Dan Kenigsberg has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 4: Code-Review+2
Francesco Romani has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 4: Continuous-Integration+1
Francesco Romani has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 4:
run CI scripts manually.
Francesco Romani has submitted this change and it was merged.
Change subject: utils: Add memoized invalidation ......................................................................
utils: Add memoized invalidation
We learned in the hard way that memoizing and monkey-patching do work nicely together. However, disabling memoizing during the tests means that we do not test the same code in the application.
This patch adds invalidate() method to memoized functions, so tests can clean up properly after they modify memozied data.
This can also be used by application code to invalidate memoized functions in certain conditions.
Change-Id: I3053b8ecc567f7c3154f7473dfd6b587823313ae Signed-off-by: Nir Soffer nsoffer@redhat.com Bug-Url: https://bugzilla.redhat.com/1267851 Reviewed-on: http://gerrit.ovirt.org/37788 Reviewed-by: Francesco Romani fromani@redhat.com Reviewed-by: Piotr Kliczewski piotr.kliczewski@gmail.com Reviewed-by: Dan Kenigsberg danken@redhat.com Reviewed-on: https://gerrit.ovirt.org/40029 Tested-by: Francesco Romani fromani@redhat.com Continuous-Integration: Francesco Romani fromani@redhat.com --- M lib/vdsm/utils.py M tests/utilsTests.py 2 files changed, 67 insertions(+), 1 deletion(-)
Approvals: Piotr Kliczewski: Looks good to me, but someone else must approve Nir Soffer: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified; Passed CI tests
automation@ovirt.org has posted comments on this change.
Change subject: utils: Add memoized invalidation ......................................................................
Patch Set 5:
* Update tracker::#1267851::OK * Set MODIFIED::bug 1267851::::#1267851::::IGNORE, not oVirt prod but vdsm
vdsm-patches@lists.fedorahosted.org