Skip to content
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

refactor: extract cache utilities #7621

Merged

Conversation

ralbertazzi
Copy link
Contributor

@ralbertazzi ralbertazzi commented Mar 7, 2023

Pull Request Check List

As discuss with @radoering in #7595, this is a first refactor PR that tries to extract cache utilities outside of Chef. The end goal is to resolve finally #2415.

  • Added an artifacts_cache_directory property to config.py analoguous to repository_cache_directory
  • Made get_cached_archives_for_link() independent from interpreter_name and interpreter_version
  • Moved get_cached_archives_for_link and get_cache_directory_for_link from chef.py to utils/cache.py

I ended up moving the recently touched get_cached_archive_for_link too, although it looks to me that it's not such a generic utility, but it's mostly related to the installation step. The fact that I had to refactor a bit the installer package to avoid circular imports makes this evident. Still, I believed chef.py wasn't the best place to keep that function since the other ones have been moved. I'm happy to receive feedback on this and other possible solutions. Most likely I would put get_cached_archive_for_link directly inside Executor( which is the only user of that function), but I wanted to get your feedback first.

  • Added tests for changed code.
  • [ ] Updated documentation for changed code.

@ralbertazzi ralbertazzi force-pushed the feat/cache-url-dependencies branch 4 times, most recently from 43171ac to 3c08430 Compare March 9, 2023 16:55
@ralbertazzi
Copy link
Contributor Author

I went ahead at moving get_cached_archive_for_link inside Executor

src/poetry/config/config.py Outdated Show resolved Hide resolved
src/poetry/installation/executor.py Outdated Show resolved Hide resolved
src/poetry/utils/cache.py Outdated Show resolved Hide resolved
@ralbertazzi ralbertazzi requested a review from radoering March 13, 2023 17:17
@radoering radoering force-pushed the feat/cache-url-dependencies branch from 44c0f95 to abff1a2 Compare March 19, 2023 13:25
@radoering radoering merged commit 36ca327 into python-poetry:master Mar 19, 2023
@ralbertazzi ralbertazzi deleted the feat/cache-url-dependencies branch March 20, 2023 07:54
adriangb pushed a commit to adriangb/poetry that referenced this pull request Apr 5, 2023
Copy link

github-actions bot commented Mar 3, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants