Fix race condition for import of modules which use apipkg in Python 3.3+ #27
Conversation
GlenWalker
force-pushed
the
fix_py3_import_race
branch
2 times, most recently
from
e68ae61 to
15b5526
Compare
* Update existing module in-place rather than replacing in sys.modules with a new ApiModule instance
GlenWalker
force-pushed
the
fix_py3_import_race
branch
from
15b5526 to
1be7d47
Compare
GlenWalker
force-pushed
the
fix_py3_import_race
branch
from
d330845 to
ed7adce
Compare
|
yikes ^^ |
|
It looks like there was an "Internal Server Error" while running the GitHub actions for your merge, so they never finished. Is it possible to re-run them? |
| delattr(mod, name) | ||
|
|
||
| # Updating class of existing module as per importlib.util.LazyLoader | ||
| mod.__class__ = ApiModule |
There was a problem hiding this comment.
This breaks the compatibility with Python 3.4: #29.
There was a problem hiding this comment.
This is unfortunate, I'm not sure there's a way to fix the race condition if we can't update the module in place.
Options:
- Figure out a way to update the module in place in Python <= 3.4, maybe by copying from ApiModule into module instance?
- Accept the race condition in Python <= 3.4 and don't try to update in place
- Drop support for Python 3.4 completely, and update python_requires appropriately
I'm not sure what @RonnyPfannschmidt's intention was for 3.4 support, I just relied on the automated tests.
There was a problem hiding this comment.
I wonder if setattr() would somehow work.
There was a problem hiding this comment.
@GlenWalker dropping 3.4 from the matrix was accidental: #31
There was a problem hiding this comment.
I wonder if setattr() would somehow work.
Setting attributes directly on the original module instance would probably work for eager=True, but not for eager=False. I had a quick look, but so far the only options for lazy loading require support from the module class (i.e. Python calls __getattr__ from the module class, and not the module instance). We wouldn't want to update __getattr__ on types.ModuleType globally, so if we can't update __class__ on the module instance then we are stuck.
I suspect that for Python 3.4 we'll just have to accept the race condition, and replace the module, rather than updating it in place. Unless anyway has any good ideas?
(The race condition has existed for a long time without anyone complaining, so this might be acceptable)
Update existing module in-place rather than replacing in sys.modules with a new ApiModule instance
This race condition exists for import statements (and __import__) in Python 3.3+ where sys.modules is checked before obtaining an import lock, and for importlib.import_module in Python 3.11+ for the same reason.