-
-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
3.8.0: dictionary changed size during iteration iterating on importlib_metadata.distributions() #293
Comments
Confirmed. All our CI pipeline unittests fail due to the new version: https://gitlab.com/secml/secml/-/pipelines/277431124 This is independent of the Python version. |
Thanks for the report. I can't tell from the traceback what's happening, and the issue isn't happening in my CI environments that also rely on importlib_metadata 3.8 and pluggy. Moreover, when looking at #290, I don't see anything in particular where a dictionary size might be altered. There is this line, but the Can someone help distill the issue into a reproducer I can use to troubleshoot? |
I'm able to reproduce the issue by checking out secml and running tox -e py36. |
If I disable the diff --git a/importlib_metadata/__init__.py b/importlib_metadata/__init__.py
index 11b9aac..1b95c8f 100644
--- a/importlib_metadata/__init__.py
+++ b/importlib_metadata/__init__.py
@@ -648,7 +648,6 @@ class FastPath:
def mtime(self):
with contextlib.suppress(OSError):
return os.stat(self.root).st_mtime
- self.lookup.cache_clear()
@method_cache
def lookup(self, mtime):
diff --git a/importlib_metadata/_functools.py b/importlib_metadata/_functools.py
index 73f50d0..632292f 100644
--- a/importlib_metadata/_functools.py
+++ b/importlib_metadata/_functools.py
@@ -75,7 +75,7 @@ def method_cache(method, cache_wrapper=None):
def wrapper(self, *args, **kwargs):
# it's the first call, replace the method with a cached, bound method
bound_method = types.MethodType(method, self)
- cached_method = cache_wrapper(bound_method)
+ cached_method = bound_method
setattr(self, method.__name__, cached_method)
return cached_method(*args, **kwargs)
|
If I switch to a different method of memoization of that method, the problem remains: diff --git a/importlib_metadata/__init__.py b/importlib_metadata/__init__.py
index 11b9aac..8649630 100644
--- a/importlib_metadata/__init__.py
+++ b/importlib_metadata/__init__.py
@@ -21,7 +21,7 @@ from ._compat import (
Protocol,
)
-from ._functools import method_cache
+from ._functools import memoize
from ._itertools import unique_everseen
from configparser import ConfigParser
@@ -648,9 +648,8 @@ class FastPath:
def mtime(self):
with contextlib.suppress(OSError):
return os.stat(self.root).st_mtime
- self.lookup.cache_clear()
- @method_cache
+ @memoize
def lookup(self, mtime):
return Lookup(self)
diff --git a/importlib_metadata/_functools.py b/importlib_metadata/_functools.py
index 73f50d0..f678ce0 100644
--- a/importlib_metadata/_functools.py
+++ b/importlib_metadata/_functools.py
@@ -83,3 +83,44 @@ def method_cache(method, cache_wrapper=None):
wrapper.cache_clear = lambda: None
return wrapper
+
+
+# from
+# http://code.activestate.com/recipes/577452-a-memoize-decorator-for-instance-methods/
+class memoize:
+ """cache the return value of a method
+
+ This class is meant to be used as a decorator of methods. The return value
+ from a given method invocation will be cached on the instance whose method
+ was invoked. All arguments passed to a method decorated with memoize must
+ be hashable.
+
+ If a memoized method is invoked directly on its class the result will not
+ be cached. Instead the method will be invoked like a static method:
+ class Obj(object):
+ @memoize
+ def add_to(self, arg):
+ return self + arg
+ Obj.add_to(1) # not enough arguments
+ Obj.add_to(1, 2) # returns 3, result is not cached
+ """
+ def __init__(self, func):
+ self.func = func
+
+ def __get__(self, obj, objtype=None):
+ if obj is None:
+ return self.func
+ return functools.partial(self, obj)
+
+ def __call__(self, *args, **kw):
+ obj = args[0]
+ try:
+ cache = obj.__cache
+ except AttributeError:
+ cache = obj.__cache = {}
+ key = (self.func, args[1:], frozenset(kw.items()))
+ try:
+ res = cache[key]
+ except KeyError:
+ res = cache[key] = self.func(*args, **kw)
+ return res That indicates that it's the memoization of the lookup object that's implicated in the failure. |
The only change between 3.7.3 and 3.8.0 is the caching (v3.7.3...v3.8.0). |
I've figured out the issue. It's in how Lookup.search uses lazy evaluation of self.info and how
|
Gotcha. |
I've got a workaround going out in 3.8.1. I still want to produce a test that captures the failure, but I wanted to get the fix out asap. |
I just tested your fix and it solves my problem. |
Sorry for the inconvenience. |
No worries, thanks for fixing it so quickly. |
3.8 is breaking my CI Link.
For now I will pin to 3.7.3, but you might want to investigate it.
Exception:
The text was updated successfully, but these errors were encountered: