Optional cached imports - #72
LourensVeen merged 9 commits into
Conversation
This test checks that some warning is logged when multiple entry points are found for one module. However, this part of the code is never reached if on the second call with reuse_cached_imports = True instead of False. That is because the entrypoints are cached in the previous call, which skips _load_from_entrypoints where the warning is raised. Therefore ymmsl_cache is cleared before and after each test to ensure a clean slate. The failed test:: FAILED ymmsl/v0_2/tests/test_resolver.py::test_resolve_entrypoints_duplicate_name[True] - assert 0 == 1 + where 0 = len([])
LourensVeen
left a comment
LourensVeen
left a comment
There was a problem hiding this comment.
Thanks, that's a good addition. I'd like to simplify the test a bit, as indicated. And could you fix the formatting so the CI passes?
LourensVeen
left a comment
LourensVeen
left a comment
There was a problem hiding this comment.
Sorry, two more nits to pick but then we'll merge it.
| from ymmsl.v0_2.configuration import Configuration | ||
| from ymmsl.v0_2.identity import Reference | ||
| from ymmsl.v0_2.resolver import resolve as resolve_impl, ymmsl_cache | ||
| from ymmsl.v0_2.resolver import resolve as resolve_impl |
There was a problem hiding this comment.
This rename is now no longer necessary, and potentially confusing. Could you remove it?
| resolve_impl( | ||
| Reference("test_resolve_imports"), | ||
| config, | ||
| reuse_cached_imports=reuse_cached_imports, |
There was a problem hiding this comment.
If we were setting this to True then passing the argument with its name makes sense because it clarifies what is true, but if we already have the variable then this is just needless duplication.
|
The CI failure is unrelated to this PR, I need to remove the Codacy coverage upload since we're not using Codacy anymore. Merging anyway. |
2 participants
The ymmsl_cache assumes that the YMMSL_PATH and sys.path does not change between resolve calls, but in fact it can.
If it does, the cached module imports can become stale.
Therefore I have added this contract explicitly in the doc-string of the resolve method, along with an option to clear the cache before resolving.