pypi downloads blocked by experimental_downloader_config fail silently

[keith]

🐞 bug report

Affected Rule

pip.parse with the bazel downloader enabled

Is this a regression?

not sure

Description

with some config like this:

pip.parse(
    experimental_extra_index_urls = [
        "https://download.pytorch.org/whl/cu121",
    ],
    experimental_index_url = "https://pypi.org/simple",
    experimental_index_url_overrides = {
        "torch": "https://download.pytorch.org/whl/cu121",
        "torchaudio": "https://download.pytorch.org/whl/cu121",
        "torchvision": "https://download.pytorch.org/whl/cu121",
    },
...
)

If your remote downloader config doesn't allow pytorch.org, rules_python can fail with this error:

Error in fail: Failed to download metadata for ["torch", "torchaudio", "torchvision"] for from urls: ["https://pypi.org/simple", "https://download.pytorch.org/whl/rocm6.2"]

Instead of the normal:

Error in download: java.io.IOException: Configured URL rewriter blocked all URLs: [https://download.pytorch.org/whl/rocm6.2/torch/]

I believe this is because of allow_fail = True https://github.com/bazelbuild/rules_python/blob/fcf7221c1e079307ff13d32239b7782d2f1dc48c/python/private/pypi/simpleapi_download.bzl#L208 and it makes it hard to spot that it's a misconfiguration on your downloader config.

🌍 Your Environment

Operating System:

linux aarch64

Output of bazel version:

95bbed03758a7651916d0708a892f5342a0421c5

Rules_python version:

fcf7221

[aignas]

I think this is working as intended. If the allow_fail is not there then we would not be able to handle extra_index_urls. Maybe we should set it to allow_fail = False if the overrides are passed?

[keith]

Yea I wasn't suggesting removing that, but we probably need custom error handling because that isn't set

[AFMiziara]

How can we enable bazel downloader in pip parser? So we can use r.g. tweag credential-helper utility?

[aignas]

Please read our docs.

[aignas]

What I realized about this issue during doing #3654 is that we could store the Overrides that the user would need to input as facts and that way we could set to a different failure expectation if we have the overrides. What is more, we could probably construct a canonical_key for caching the response based on the requested versions, but that would require more refactoring.

[aignas]

The index will provide <index_url>/simple/ with the list of packages that it has. We could use this to first check which packages we may be able to download and then we would not need the allow_fail=True at all. This is probably the best solution.

EDIT: unassigning myself in case anyone ones to take a stab at it.