Use locally cached wheels during install #5871
Conversation
| # If the archive is already a wheel, there is no need to cache it. | ||
| if link.is_wheel: | ||
| return link |
There was a problem hiding this comment.
Considering the comment, I can only imagine that might be a shortcut for local wheels? Maybe, you can examine if the change has some side effects on wheels as path dependencies. 🙏
There was a problem hiding this comment.
The only place this is called is in the executor _download_link() code path, which means that it is never relevant for path dependencies.
I'm quite unsure about what this is supposed to be doing - Chef isn't a name that helps much. At first I'd thought it was going to be some general purpose artifact cache, but its extremely limited scope is puzzling to me.
There was a problem hiding this comment.
I need to look at this when I have more time. But I suspect the original intent was to cache the environment specific when built. Useful when the link is an sdist.
dimbleby
force-pushed
the
cached-wheels
branch
3 times, most recently
from
2847e9a
to
d34eae4
Compare
|
As a sort-of convincer that it's fine to use the cache for wheels: this is what poetry 1.1.13 has been doing all along. The comment and the code were mismatched prior to #4967, which jumped in the direction of favouring the comment. But this MR suggests that's the wrong way to jump, and the fact that poetry 1.1.13 had been "wrong" all that time is some sort of evidence that it's safe to undo that. |
|
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. |
Related to #2415, though it only addresses one of the many repeated downloads which that issue reports.
If we have a url dependency pointing at a wheel eg
then poetry performs the download during
poetry install, even though it has a perfectly good cached version in~/.cache/pypoetry/artifacts.get_cached_archive_for_linklooks kinda confused: returning the original inputLinkto mean "no cached archive found" is odd. So I have tweaked it to returnNoneinstead which seems a lot clearer.It's always possible that I am the confused one, certainly I am struggling to understand what the intention of the changed code can have been. So do please think about that when reviewing!