fix: fix exception on invalid url dependency for current env

[ralbertazzi]

The Executor first downloads and caches the original artifact, then tries to fetch a higher priority cached artifact (i.e. a wheel instead of a sdist) to speed up the installation, falling back on the previously downloaded original artifact. Therefore it was assumed that whatever cached artifact gets returned, it will not be None.

The assumption however is wrong since the cached artifact search is filtered for the given environment, i.e. if even the original artifact is invalid for the current environment than artifact will actually be None, raising a quite mysterious exception. This can happen for instance in this case:

[tool.poetry.dependencies]
python = "~3.9"

# Requesting a Python 3.10 wheel on a Python 3.9 environment
torch = { url = "https://download.pytorch.org/whl/torch-1.13.1-cp310-cp310-manylinux2014_aarch64.whl" }

The fix simply transforms that mysterious exception into a more self-explanatory one.

[dimbleby]

this isn't a ValueError, no-one has provided an unsuitable value anywhere.

Really it's just a plain bug: if left unfixed then suggest that RuntimeError is more accurate.

[ralbertazzi]

@dimbleby I think I didn't get what bug exactly are you referring to, isn't this a user error?

[radoering]

Same here. Do you think it's a bug that it's possible to lock something like this and the error only pops up in the executor?

[ralbertazzi]

While technically you could avoid locking my example in the first place, I think there's other situations in which you really can't know until you install (different os, different arch, ..), so I think this error handling is still valid. Plus, it feels consistent with the current "lock everything" behaviour of Poetry.

[dimbleby]

I definitely think that this error popping up in download_link() is strange.

_download_link() seems confused about what it is for. It mixes up the straightforward "download an archive, possibly getting it from cache" with some sort of "but actually maybe not the archive that I was asked for, and I'll check compatibility with the current environment, but only if I look in the cache". I'm not sure how best to disentangle this: but it certainly is not pretty.

I'm fine with this MR being an incremental improvement: but it's very strange to me that checking an archive for suitability for the current environment happens (i) during download and (ii) only if the archive was in the cache.

(What's the behaviour in that repro with --no-cache? Perhaps this cache just doesn't respect --no-cache...)

[ralbertazzi]

Perhaps this cache just doesn't respect --no-cache

It doesn't. #7693 (comment)

And definitely agree on the behaviour of _download_link being sketchy

[radoering]

I agree that _download_link() does more than you would expect it to do but since that has already been the case in 1.4, we can defer cleaning this up.

(ii) only if the archive was in the cache

The check is always executed, no matter if the artifact was in the cache before. If it was not in the cache it is downloaded and put into the cache before this check.

[github-actions]

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.