Teach packages.LocalPackageLookup to look at other venvs
#215
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Thanks! Still have a fair amount of wrangling with Mypy (and Pylint, I suspect) to do. But I like the core structure of this. |
jherland
force-pushed
the
jherland/look-at-other-venv
branch
from
a3c3f93 to
6ba869e
Compare
jherland
changed the title
packages.LocalPackageLookup to look at other venvs
Nour-Mws
approved these changes
Mar 14, 2023
We're about to start relying on internal functions from importlib_metadata. Although these are available via the stdlib module importlib.metadata in Python v3.11, we are likely keep relying on upcoming bugfixes from the importlib_metadata project, and it is probably safer for us to _pin_ this 3rd-party dependency, so that we can avoid breakage if/when its internal functions change.
We now depend on importlib_metadata for all of our supported Python versions, so there's no need to differentiate between these imports.
This builds the base for making FawltyDeps able to find dep -> import name mappings in a different venv to where FD itself is runnning, including being able to peek at a virtualenv associated with a different Python version (within reason). We use some internal functions and functionality from importlib_metadata to decouple ourselves from the current sys.path (and sys.meta_path), and instead peek into the site-packages directory of another virtualenv directory. This commit adds the LocalPackageLookup API to fo this "remote" lookup, and also adds a few basic tests to verify this new functionality.
LocalPackageLookup recently learned how to peek into another venv and find its installed packages. Use this feature in test_real_projects to verify that the requirements declared for the current Experiment are in fact present, before we proceed with testing the expected result from running FD on this Experiment. This is a relatively cheap way, both to verify that our cached venvs are OK, and to test that LocalPackageLookup in a different venv works correctly.
When we added the venv_path argument to LocalPackageLookup, we retained a sanity check for the default venv_path=None case, where we compared the new method of building .packages against the old method method (using packages_distributions()), and aborted if these were not 100% equivalent. After adding a test to test_real_projects, we have now verified that this sanity check holds for all of the venvs we use as part of our real_projects test. Since the sanity check has not triggered, we can conclude that these methods are in fact equivalent, and we can drop this sanity check.
jherland
force-pushed
the
jherland/look-at-other-venv
branch
from
6ba869e to
e75553d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the most basic building block towards decoupling the venv that FawltyDeps is installed in, from the venv that FawltyDeps is run against. From this we can work towards several goals in future PRs:
pip install fawltydepsinto your project's venv, but instead e.g.pipx install fawltydepsto make FD available to all your projects.--venvas a command line optionbasepath(i.e. make--venvsomewhat similar to--codeand--deps)real_projectstests to no longer have to install/uninstall FD in the experiment's venv.This PR is directly relevant to #161, and as such is also related to task 2 in the roadmap: #195. As far as #142 is still relevant, I suspect this might go a way towards fixing that as well. Furthermore, revisiting #187 is probably warranted after this PR is merged.
Commits:
pyproject.toml: Use latestimportlib_metadataacross all Python versionsimportlib_metadatainstead ofimportlib.metadataeverywherepackages: TeachLocalPackageLookupto start looking at other venvstest_real_projects: Verify that all requirements are presentpackages.LocalPackageLookup: Remove redundant sanity check