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

[Feature Request] Improved logging for failures #21429

Open
logan-markewich opened this issue Sep 18, 2024 · 12 comments
Open

[Feature Request] Improved logging for failures #21429

logan-markewich opened this issue Sep 18, 2024 · 12 comments
Assignees

Comments

@logan-markewich
Copy link

Over at llama-index, we are huge users of pants, using it to test across 500+ python packages in a monorepo

Quite often, some package will depend on some external python package that updates and breaks things.

One thing I've noticed is that issues like envs failing to solve, or issues around interpreter constraints, are often very hard to debug.

When failing to solve venvs, usually I can read the dependencies and figure out which packages it might be failing on, but the actual name of the package or test its trying to build isn't reported in the error.

More recently, I hit some issues with interpreter constraints, and the error is even more vague

ProcessExecutionFailure: Process 'Find interpreter for constraints: CPython==3.10.*,==3.11.* OR CPython==3.10.*,==3.12.* OR CPython==3.11.* OR CPython==3.11.*,==3.12.* OR CPython==3.11.*,==3.9.* OR CPython==3.12.* OR CPython==3.12.*,==3.9.*' failed with exit code 102.

stdout:

stderr:
Could not find a compatible interpreter.

Examined the following interpreters:
1.) /home/runner/.cache/pypoetry/virtualenvs/llama-index-MknZL8ZV-py3.10/bin/python CPython==3.10.15
2.)                                                             /usr/bin/python3.10 CPython==3.10.12

No interpreter compatible with the requested constraints was found:
  Version matches CPython==3.10.*,==3.11.* or CPython==3.10.*,==3.12.* or CPython==3.11.* or CPython==3.11.*,==3.12.* or CPython==3.11.*,==3.9.* or CPython==3.12.* or CPython==3.12.*,==3.9.*

Use `--keep-sandboxes=on_failure` to preserve the process chroot for inspection.

make: *** [Makefile:14: test] Error 1

While the error is clear enough, what isn't clear is where and what package/tests in my monorepo is causing the issue 😅

What I would like to see is just, when something goes wrong, pants doing its best to report what/where the issue is

@logan-markewich logan-markewich changed the title Improved logging for failures [Feature Request] Improved logging for failures Sep 18, 2024
@jsirois
Copy link
Contributor

jsirois commented Sep 18, 2024

Well, I can't speak for Pants, but the underlying error message here comes from Pex and it does assume you know what comma means in the Python ecosystem (it means AND 1). Knowing that, here's the breakdown:

  1. impossible: CPython==3.10.*,==3.11.*
  2. impossible: CPython==3.10.*,==3.12.*
  3. CPython==3.11.*
  4. impossible: CPython==3.11.*,==3.12.*
  5. impossible: CPython==3.11.*,==3.9.*
  6. CPython==3.12.*
  7. impossible: CPython==3.12.*,==3.9.*

So of the 7 OR'd interpreter constraints, 5 are impossible to ever satisfy. You can't have a Python interpreter that is both version 3.10 and version 3.11 (example item 1) at the same time. That's impossible! Maybe you meant to write CPython>=3.10,<3.12 for item 1? So you are left with 2 valid interpreter constraints in 3 and 6 leaving:
CPython==3.11.* OR CPython==3.12.* and the interpreters identified in the error message are both 3.10 and so, of course, don't match.

Maybe, though, by "While the error is clear enough," you mean you understood this part of things? If so though, it's weird you include that example without correcting the 5 invalid ICs in the repo, which you could presumably find without more info by simply grepping pants.toml + BUILD files.
For example, this: https://github.com/run-llama/llama_index/blob/044a439dc2fda53c991e49a43e4e8e652dd8a735/pants.toml#L22
Should probably be just:

[python]
interpreter_constraints = [">=3.9,<3.13"]

It means the same thing and avoids the Python ecosystem non-standard OR you get when you make a list of these things in Pants.

Footnotes

  1. Search for "The comma (“,”) is equivalent to a logical and operator" here: https://peps.python.org/pep-0440/#version-specifiers

@jsirois
Copy link
Contributor

jsirois commented Sep 18, 2024

Yeah, Pants is probably doing incorrect algebra somewhere, it looks like you never actually write CPython==3.10.*,==3.11.* anywhere in the repo. You might want to correct all of these to eliminate OR (2 or more items in IC list, and have all use a 1-item IC list that leverage the standard Python AND (,) operator as discussed above. That might work around what appear to be Pants bugs synthesizing interpreter constraints:

:; git grep interpreter_constraint -- '**/BUILD' | grep '",'
llama-index-integrations/embeddings/llama-index-embeddings-mistralai/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/embeddings/llama-index-embeddings-vertex/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/evaluation/llama-index-evaluation-tonic-validate/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/extractors/llama-index-extractors-marvin/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/indices/llama-index-indices-managed-google/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/indices/llama-index-indices-managed-vertexai/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/llms/llama-index-llms-litellm/tests/BUILD:  interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/llms/llama-index-llms-llama-api/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/llms/llama-index-llms-vertex/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/program/llama-index-program-guidance/tests/BUILD:    interpreter_constraints = ["==3.9.*", "==3.10.*"],
llama-index-integrations/question_gen/llama-index-question-gen-guidance/tests/BUILD:    interpreter_constraints = ["==3.9.*", "==3.10.*"],
llama-index-integrations/readers/llama-index-readers-firebase-realtimedb/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/readers/llama-index-readers-firestore/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/readers/llama-index-readers-gcs/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/readers/llama-index-readers-lilac/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/readers/llama-index-readers-pandas-ai/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/readers/llama-index-readers-sec-filings/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/readers/llama-index-readers-whatsapp/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/response_synthesizers/llama-index-response-synthesizers-google/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/retrievers/llama-index-retrievers-vertexai-search/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/storage/docstore/llama-index-storage-docstore-firestore/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/storage/index_store/llama-index-storage-index-store-firestore/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/storage/kvstore/llama-index-storage-kvstore-firestore/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/tools/llama-index-tools-google/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/tools/llama-index-tools-text-to-image/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/vector_stores/llama-index-vector-stores-deeplake/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-integrations/vector_stores/llama-index-vector-stores-lancedb/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"]
llama-index-integrations/vector_stores/llama-index-vector-stores-vertexaivectorsearch/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-packs/llama-index-packs-deeplake-deepmemory-retriever/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-packs/llama-index-packs-deeplake-multimodal-retrieval/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-packs/llama-index-packs-gmail-openai-agent/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-packs/llama-index-packs-panel-chatbot/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-packs/llama-index-packs-ragatouille-retriever/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-packs/llama-index-packs-vanna/tests/BUILD:    interpreter_constraints=["==3.9.*", "==3.10.*"],
llama-index-packs/llama-index-packs-zenguard/tests/BUILD:  interpreter_constraints=["==3.9.*", "==3.10.*"],

@logan-markewich
Copy link
Author

@jsirois thanks for the tip (and I didn't mean to come off as rude or anything btw, no hostility here 😁 )

Fumbling interpreter constraints aside, maybe a better example to illustrate
https://github.com/run-llama/llama_index/actions/runs/10925018699/job/30325491519

This timed out trying to solve dependencies. Totally fair, I think some python version on something changed, causing this to timeout. But from there error, it's not indicated what test or package this was building for. I was able to guess based on the deps it was installing, but it's not always that easy.

I'm not sure about the underlying implementation details on how pants interacts with pex, just felt like when things go wrong at this stage, it might be possible to report what/where.

In any case, if that's not possible, feel free to close this

@jsirois
Copy link
Contributor

jsirois commented Sep 19, 2024

For that error, you're seeing the direct output of Pip. There is really nothing more Pex or Pants could do for that one, except, as you allude to, maybe blame Pip. It was an explicit goal of Pex to kill it's bespoke resolver years ago in favor of just using Pip. Presumably the gold standard, but agreed in advance, the situation in reality is a mess.

You can ensure you're using the latest Pip (--pip-version latest when using Pex, not sure how you control this from Pants). Lots of example issues over in Pip about this, but here's one: pypa/pip#12754

@logan-markewich
Copy link
Author

Right, definitely an issue with pip.

I'm guessing there's no way for pants to report what test/package/path caused pip to break?

@jsirois
Copy link
Contributor

jsirois commented Sep 19, 2024

Nope. If you read the issue I pointed to, even Pip folks aren't sure yet!

@jsirois
Copy link
Contributor

jsirois commented Sep 19, 2024

I have no say on this issue, I maintain Pex, not Pants. But, if I were you I'd at least push hard on Pants killing interpreter constraint OR support. It's non-standard and confusing and, I think, handled buggily in cases like yours. Unlike Pex, Pants typically has no problems deprecating and removing features; so I'd think it could remove this foot gun.

@jsirois
Copy link
Contributor

jsirois commented Sep 19, 2024

I've been doing a general sweep over Pex issues over the last few months and found pex-tool/pex#432 which would have Pex fail fast when Pants handed it something like ==3.11.*,==3.12.* or else warn in the case you presented in the OP where 5 out of the 7 constraints were impossible but 2 were valid. Pex has since grown the technology to make that sort of determination; so I'll pick that up and get out a Pex release with that enhancement.

@jsirois
Copy link
Contributor

jsirois commented Sep 19, 2024

@logan-markewich for the OP interpreter constraint message you'll now see the following when Pants upgrades to Pex 2.20.1 (or you do): https://github.com/pex-tool/pex/pull/2542/files#diff-5d77c74e67c3b985b12a434cde0c358f59c3e381f4cb033e86e7a2046048c018R57-R80 ... assuming Pants shows warning stderr output from tools it calls. Hopefully that's better.

@benjyw benjyw self-assigned this Sep 19, 2024
@benjyw
Copy link
Contributor

benjyw commented Sep 30, 2024

I have no say on this issue, I maintain Pex, not Pants. But, if I were you I'd at least push hard on Pants killing interpreter constraint OR support. It's non-standard and confusing and, I think, handled buggily in cases like yours. Unlike Pex, Pants typically has no problems deprecating and removing features; so I'd think it could remove this foot gun.

FWIW the new Python backend will not allow OR of interpreter constraints. In fact it will probably require you to strictly lock down the specific interpreters you use by requiring complete-platform information rather than a loose interpreter version glob. While I generally don't want to nanny users too much, this particular footgun needs to be tamed.

@benjyw
Copy link
Contributor

benjyw commented Sep 30, 2024

For that error, you're seeing the direct output of Pip. There is really nothing more Pex or Pants could do for that one, except, as you allude to, maybe blame Pip. It was an explicit goal of Pex to kill it's bespoke resolver years ago in favor of just using Pip. Presumably the gold standard, but agreed in advance, the situation in reality is a mess.

You can ensure you're using the latest Pip (--pip-version latest when using Pex, not sure how you control this from Pants). Lots of example issues over in Pip about this, but here's one: pypa/pip#12754

FWIW this is how you set the underlying pip version in Pants: https://www.pantsbuild.org/stable/docs/python/overview/pex#setting-pex-and-pip-versions

@jsirois
Copy link
Contributor

jsirois commented Oct 2, 2024

this particular footgun needs to be tamed.

In case it wasn't clear, I also initially thought this was a footgun; i.e.: I thought @logan-markewich mistakenly wrote down an IC of "CPython==3.11.*,==3.12.*" somewhere. I looked at https://github.com/run-llama/llama_index though and found that IC was not written down anywhere in the repo. Although the OR syntax Pants supports is used for ICs in the repo, it looks to be used correctly. I think there is a Pants bug that is concocting the invalid ICs listed in the OP error message. For reference, that was:
ProcessExecutionFailure: Process 'Find interpreter for constraints: CPython==3.10.*,==3.11.* OR CPython==3.10.*,==3.12.* OR CPython==3.11.* OR CPython==3.11.*,==3.12.* OR CPython==3.11.*,==3.9.* OR CPython==3.12.* OR CPython==3.12.*,==3.9.*' failed with exit code 102..

So, IIUC, there are two issues there:

  1. The very old issue of Pants supporting OR at all (Twitter added this support, I think out of ignorance of the standard (if awkward) !=X.Y.*,... technique for excluding range middles).
  2. What appears to be a Pants bug processing ORs. I say Pants because Pex does ~no OR processing, but Pants decidedly does:
    @classmethod
    def merge_constraint_sets(
    cls, constraint_sets: Iterable[Iterable[str]]
    ) -> frozenset[Requirement]:
    """Given a collection of constraints sets, merge by ORing within each individual constraint
    set and ANDing across each distinct constraint set.
    For example, given `[["CPython>=2.7", "CPython<=3"], ["CPython==3.6.*"]]`, return
    `["CPython>=2.7,==3.6.*", "CPython<=3,==3.6.*"]`.
    """
    # A sentinel to indicate a requirement that is impossible to satisfy (i.e., one that
    # requires two different interpreter types).
    impossible = parse_constraint("IMPOSSIBLE")
    # Each element (a Set[ParsedConstraint]) will get ANDed. We use sets to deduplicate
    # identical top-level parsed constraint sets.
    # First filter out any empty constraint_sets, as those represent "no constraints", i.e.,
    # any interpreters are allowed, so omitting them has the logical effect of ANDing them with
    # the others, without having to deal with the vacuous case below.
    constraint_sets = [cs for cs in constraint_sets if cs]
    if not constraint_sets:
    return frozenset()
    parsed_constraint_sets: set[frozenset[Requirement]] = set()
    for constraint_set in constraint_sets:
    # Each element (a ParsedConstraint) will get ORed.
    parsed_constraint_set = frozenset(
    parse_constraint(constraint) for constraint in constraint_set
    )
    parsed_constraint_sets.add(parsed_constraint_set)
    if len(parsed_constraint_sets) == 1:
    return next(iter(parsed_constraint_sets))
    def and_constraints(parsed_constraints: Sequence[Requirement]) -> Requirement:
    merged_specs: set[tuple[str, str]] = set()
    expected_interpreter = parsed_constraints[0].project_name
    for parsed_constraint in parsed_constraints:
    if parsed_constraint.project_name != expected_interpreter:
    return impossible
    merged_specs.update(parsed_constraint.specs)
    formatted_specs = ",".join(f"{op}{version}" for op, version in merged_specs)
    return parse_constraint(f"{expected_interpreter}{formatted_specs}")
    ored_constraints = (
    and_constraints(constraints_product)
    for constraints_product in itertools.product(*parsed_constraint_sets)
    )
    ret = frozenset(cs for cs in ored_constraints if cs != impossible)
    if not ret:
    # There are no possible combinations.
    attempted_str = " AND ".join(f"({' OR '.join(cs)})" for cs in constraint_sets)
    raise ValueError(
    softwrap(
    f"""
    These interpreter constraints cannot be merged, as they require
    conflicting interpreter types: {attempted_str}
    """
    )
    )
    return ret

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

No branches or pull requests

3 participants