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

Always sort installed records and solver specs #378

Merged
merged 9 commits into from
Nov 14, 2023
Merged

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Nov 13, 2023

Description

I've been thinking about some failures we get (only!) sometimes in CI, like tests/core/test_solve.py::test_conda_downgrade[libmamba] (see #317). These errors often go away with a retry in the same PR, so there's some randomness at play.

The hypothesis here is that some container is getting a different order depending on the attempt. SolverInputState.installed takes whatever order appears in PrefixData._prefix_records, which blindly "takes" whatever os.scandir() is returning. This function does not guarantee order, so we are going to switch to a toposorted installed container. Maybe this makes the tests behave more reliably?

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Nov 13, 2023
@jaimergp jaimergp changed the title Always toposort installed records Always sort installed records Nov 13, 2023
@jaimergp jaimergp marked this pull request as ready for review November 14, 2023 15:07
@jaimergp
Copy link
Contributor Author

Seems to work. The errors we saw in the last run (first attempt) are unrelated (#334, and a pip issue with classic that smells like rm_rf race condition).

See #317 for the whole debugging story.

@jaimergp jaimergp requested a review from jezdez November 14, 2023 15:23
@jaimergp jaimergp changed the title Always sort installed records Always sort installed records and solver specs Nov 14, 2023
@jaimergp jaimergp merged commit 25d2161 into main Nov 14, 2023
69 of 71 checks passed
@jaimergp jaimergp deleted the sort-installed branch November 14, 2023 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants