pex-tool / pex

A tool for generating .pex (Python EXecutable) files, lock files and venvs.
https://docs.pex-tool.org
Apache License 2.0
2.49k stars 254 forks source link

give package details when failed parsing requirements #2440

Closed jasonwbarnett closed 2 weeks ago

jasonwbarnett commented 2 weeks ago

When I was trying to resolve a lockfile in pants I kept getting an error like this:

InvalidRequirement: .* suffix can only be used with `==` or `!=` operators
    scikit-learn (>=1.0.*) ; extra == 'pipelines'

It was very difficult to understand what to do or how to even begin to get to root cause. I ended up hacking up the cached pex source to get the relevant details, i.e. project name and version. This PR aims to reduce the cost of getting the information necessary to work through an error like this.

This could probably be expanded to any exception, but I know that's a code smell so I'm not sure, but I can imagine there are more errors like this that could be challenging to debug for end users.

jsirois commented 2 weeks ago

Thanks @jasonwbarnett. A test would be great, but I'm happy to circle back and add it as well. I'll wait to kick off CI until you have fixed the 2 Python 2.7 breaks I pointed out above.

You might want to ./dtox.sh -efmt,lint,check to at least vet your change doesn't break things in a basic way (the 1st ./dtox run will be slow - it effectively builds a dev container). Alternatively, if you have a fast internet connection, you might try BASE_MODE=pull ./dtox.sh -efmt,lint,check instead, which will pull the image from https://github.com/orgs/pex-tool/packages/container/package/pex%2Fbase.

I will note if you were using --pip-version 24.1 I think you'd get Pip failing fast and a better error message as a result today. That Pip version became supported just a few days ago in https://github.com/pex-tool/pex/releases/tag/v2.5.0.

jasonwbarnett commented 2 weeks ago

@jsirois Thank you for the quick feedback. I'll take a turn on this and get back to you later today.

I will note if you were using --pip-version 24.1 I think you'd get Pip failing fast and a better error message as a result today. That Pip version became supported just a few days ago in https://github.com/pex-tool/pex/releases/tag/v2.5.0.

I'm going to update pex in pants manually (docs) and see if the error message is better. I'll use that as additional input into this PR.

jasonwbarnett commented 2 weeks ago

@jsirois you're totally right. After adding this to pants.toml, the error message is so much better.

[python]
pip_version = "latest"

[pex-cli]
version = "v2.6.0"
known_versions = [
    "v2.6.0|macos_arm64|bc8fc449fc8165c395b26b407cc6033cf25cf02a76b7ede54828d2e899c1ee97|4157988",
    "v2.6.0|macos_x86_64|bc8fc449fc8165c395b26b407cc6033cf25cf02a76b7ede54828d2e899c1ee97|4157988",
    "v2.6.0|linux_x86_64|bc8fc449fc8165c395b26b407cc6033cf25cf02a76b7ede54828d2e899c1ee97|4157988",
    "v2.6.0|linux_arm64|bc8fc449fc8165c395b26b407cc6033cf25cf02a76b7ede54828d2e899c1ee97|4157988",
]

Error message:

WARNING: Ignoring version 1.27.0 of mlflow since it has invalid metadata:
Requested mlflow==1.27.0 from https://files.pythonhosted.org/packages/12/49/1c6f1535bb8b9f35463b043c08a902531a367ed42b0fe4afb3882bb28f8a/mlflow-1.27.0-py3-none-any.whl has invalid metadata: .* suffix can only be used with `==` or `!=` operators
    scikit-learn (>=1.0.*) ; extra == 'pipelines'
                  ~~~~~~^
Please use pip<24.1 if you need to use this version.
ERROR: Ignored the following yanked versions: 2.0.0, 2.12.0
ERROR: Ignored the following versions that require a different python version: 8.19.0 Requires-Python >=3.10; 8.20.0 Requires-Python >=3.10; 8.21.0 Requires-Python >=3.10; 8.22.0 Requires-Python >=3.10; 8.22.1 Requires-Python >=3.10; 8.22.2 Requires-Python >=3.10; 8.23.0 Requires-Python >=3.10; 8.24.0 Requires-Python >=3.10; 8.25.0 Requires-Python >=3.10
ERROR: Could not find a version that satisfies the requirement mlflow==1.27.0 (from versions: 0.0.1, 0.1.0, 0.2.0, 0.2.1, 0.3.0, 0.4.0, 0.4.1, 0.4.2, 0.5.0, 0.5.1, 0.5.2, 0.6.0, 0.7.0, 0.8.0, 0.8.1, 0.8.2, 0.9.0, 0.9.0.1, 0.9.1, 1.0.0, 1.1.0, 1.1.1.dev0, 1.2.0, 1.3.0, 1.4.0, 1.5.0, 1.6.0, 1.7.0, 1.7.1, 1.7.2, 1.8.0, 1.9.0, 1.9.1, 1.10.0, 1.11.0, 1.12.0, 1.12.1, 1.13, 1.13.1, 1.14.0, 1.14.1, 1.15.0, 1.16.0, 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.20.1, 1.20.2, 1.21.0, 1.22.0, 1.23.0, 1.23.1, 1.24.0, 1.25.0, 1.25.1, 1.26.0, 1.26.1, 1.27.0, 1.28.0, 1.29.0, 1.30.0, 1.30.1, 2.0.0rc0, 2.0.1, 2.1.0, 2.1.1, 2.2.0, 2.2.1, 2.2.2, 2.3.0, 2.3.1, 2.3.2, 2.4.0, 2.4.1, 2.4.2, 2.5.0, 2.6.0, 2.7.0, 2.7.1, 2.8.0, 2.8.1, 2.9.0, 2.9.1, 2.9.2, 2.10.0, 2.10.1, 2.10.2, 2.11.0, 2.11.1, 2.11.2, 2.11.3, 2.11.4, 2.12.1, 2.12.2, 2.13.0, 2.13.1, 2.13.2, 2.14.0rc0, 2.14.0, 2.14.1)
ERROR: No matching distribution found for mlflow==1.27.0

Based on that, I think I should just abandon this unless I'm missing something. Your thoughts?

jsirois commented 2 weeks ago

Based on that, I think I should just abandon this unless I'm missing something. You thoughts?

I think the spirit of the change is good [^1], but it probably ought to be more comprehensive. I'm happy to file an issue and take it up.

[^1]: The spirit here is don't trust Pip. The Pex vendored Pip (20.3.4) is known to do invalid resolves and newer Pips selected with --pip-version obviously behave differently from each other, tending to get better over time, but not always. Being defensive and not assuming distributions resolved by Pip are valid seems like the right thing to do given your experience.

jsirois commented 2 weeks ago

Thanks for bringing this to my attention @jasonwbarnett. I filed #2441 to track my work on this. Should have a release out by tomorrow with improvements for old --pip-versions.