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

Improve UnsatisfiableInterpreterConstraintsError. #1028

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Sep 1, 2020

Surface all broken interpreters and a link to known breaks and
workarounds tracked in #1027.

Surface all broken interpreters and a link to known breaks and
workarounds tracked in pex-tool#1027.
@jsirois jsirois mentioned this pull request Sep 1, 2020
3 tasks
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I like this compromise.

pex/interpreter_constraints.py Outdated Show resolved Hide resolved
pex/jobs.py Outdated Show resolved Hide resolved
pex/jobs.py Show resolved Hide resolved
@tdyas
Copy link
Contributor

tdyas commented Sep 1, 2020

This is good.

I would like to see this link to the pants.readme.io documentation instead of a GitHub issue. The installation problems should live near the installation documentation.

@Eric-Arellano
Copy link
Contributor

link to the pants.readme.io documentation

Those docs are for Pants, not Pex.

Pex docs are unfortunately extremely stale, as we don't have the keys to the site anymore: https://pex.readthedocs.io/en/stable/.

Benjy and I have discussed a 10% side project for me to add a dedicated Pex site for pants.readme.io. It would help Pex users and, by extension, Pants users.

@tdyas
Copy link
Contributor

tdyas commented Sep 1, 2020

Pex docs are unfortunately extremely stale, as we don't have the keys to the site anymore: https://pex.readthedocs.io/en/stable/.

Understood. I was conflating the two sites then. Separately, we should recover access to the pex site. Seems like support is done through GitHub issues: https://docs.readthedocs.io/en/stable/contribute.html#initial-triage

Somehow id(PythonInterpreter) != id(cls) on Travis interpreters.
@jsirois jsirois force-pushed the UnsatisfiableInterpreterConstraintsError/improve branch from a9a448f to ae95ab3 Compare September 11, 2020 17:09
@jsirois jsirois merged commit f44cba1 into pex-tool:master Sep 11, 2020
@jsirois jsirois deleted the UnsatisfiableInterpreterConstraintsError/improve branch September 11, 2020 19:11
def _compatible_interpreters_iter(interpreters_iter, compatibility_constraints=None):
if compatibility_constraints:
for interpreter in matched_interpreters_iter(interpreters_iter, compatibility_constraints):
for interpreter in _matched_interpreters_iter(interpreters_iter, compatibility_constraints):
yield interpreter
else:
for interpreter in interpreters_iter:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch does not handle the switch from PythonInterpreter.iter to PythonInterpreter.iter_candidates here: https://github.com/pantsbuild/pex/pull/1028/files#diff-670bc388096d1ef0c7994c4d37976392R82 so we can get error tuples mixed into the yielded interpreters when there are no compatibility constraints.

Copy link
Contributor

@Eric-Arellano Eric-Arellano Sep 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we can get error tuples

This was something I was confused by when adding type hints. In addition to the type hints, we may want to rename this to interpreter_or_failure:

https://github.com/pantsbuild/pex/blob/aae2a6ad55e148ada43f36c1d486af9b2312af50/pex/pex_bootstrapper.py#L97-L110

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem is better fixed by restructuring for locality. The code that calls PythonInterpreter.iter_candidates should deal with the consequences of that directly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not rename since we don't use Hungaian elsewhere but this confusing bit of code where typing is path-dependent is now typed in #1046

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants