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

Load all channels with conda internals #65

Merged
merged 92 commits into from
Mar 30, 2023
Merged

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Nov 14, 2022

Description

Closes #59


We are replacing the libmamba networking stack with conda internals:

  • We download all JSONs with the newest conda.SubdirData.repo_fetch interface that doesn't process the JSON (only downloads).
  • We use libmambapy.Repo, without the SubdirData indirection. This interface allow us to use a JSON file directly (without having to be optimistic about the "find the JSON file" logic in SubdirData).
  • All the tests that checked the libmamba networking stack have been removed, but the utilities are still there in case we ever need them in the future.
  • These changes also allow us to drop tons of pre- and post-processing of MatchSpec and PackageRecord objects (e.g. percentage-encoding and decoding on local paths so libcurl doesn't choke, Windows things, etc). The libmamba<->conda index interface is better abstracted now.

There's some legacy issues we should remove in the future:

  • The SubdirData subclass can introduce some issues with incomplete cache entries we have to invalidate (some tests break otherwise), because metaclass-based caches are that fun.
  • In some tests, the cached files are not found because the integration tests abuse the SubdirData._cache_ a bit too much and some of those changes are not reflected on disk. Some changes upstream will alleviate this.
  • TODO:
    • Change conda.testing.helpers._patch_for_local_exports to not patch _mtime to inf. Return early if the solver is classic because that function is not needed there.
    • Remove this block

@jaimergp
Copy link
Contributor Author

Remaining errors have to do with conda.testing.helpers._patch_for_local_exports again. Since we are using SubdirData now, the cached entries get in the way of the testing mocking machinery.

@jaimergp

This comment was marked as outdated.

@jaimergp

This comment was marked as outdated.

@jaimergp

This comment was marked as outdated.

@dholth
Copy link
Contributor

dholth commented Jan 18, 2023

We've merged a new conda that changes the way repodata.json is dowloaded, and we will change it again in the next release. Is this code compatible with the new conda? conda/conda#12050 also helps a lot avoiding wasted parsing.

@jaimergp jaimergp mentioned this pull request Jan 20, 2023
6 tasks
@jaimergp
Copy link
Contributor Author

jaimergp commented Jan 20, 2023

Nope, conda/conda#12003 changed the internal API so as a matter of facts everything will fail here as it is now. I am inclined to wait til things get calmer around the repodata fetching logic before working on this PR further. What do you think @dholth?

@jezdez
Copy link
Member

jezdez commented Jan 30, 2023

Nope, conda/conda#12003 changed the internal API so as a matter of facts everything will fail here as it is now. I am inclined to wait til things get calmer around the repodata fetching logic before working on this PR further. What do you think @dholth?

I concur, let's move this to a later milestone.

@jaimergp
Copy link
Contributor Author

tests/test_solvers.py::TestLibMambaSolver::test_arch_preferred_over_noarch_when_otherwise_equal 

This one is failing sporadically. It can be reproduced locally by rerunning it several times; it will pass 4 out of 5 times or so. I wonder if this has to do with some change in 1.4.1 (this is the first time I see it fail, if memory serves).

@jaimergp jaimergp mentioned this pull request Mar 29, 2023
3 tasks
@jaimergp
Copy link
Contributor Author

I added some stdout in the spinner (so it hides when needed; e.g. --json or --quiet), and it looks like this:

During fetch:

$ CONDA_SUBDIR=win-64 conda create -n unused --dry-run python --solver=libmamba -c ../conda-src/tests/data/legacy_repodata/ -c defaults
Channels:
 - file:///opt/conda-src/tests/data/legacy_repodata
 - defaults
 - conda-forge
Platform: win-64
Collect all metadata (repodata.json): /

Once done:

$ CONDA_SUBDIR=win-64 conda create -n unused --dry-run python --solver=libmamba -c ../conda-src/tests/data/legacy_repodata/ -c defaults
Channels:
 - file:///opt/conda-src/tests/data/legacy_repodata
 - defaults
 - conda-forge
Platform: win-64
Collect all metadata (repodata.json): done
Solving environment: done

@jezdez
Copy link
Member

jezdez commented Mar 30, 2023

Nice!

@jaimergp jaimergp marked this pull request as ready for review March 30, 2023 10:58
tests/test_user_agent.py Outdated Show resolved Hide resolved
@jezdez jezdez merged commit 700d0d4 into main Mar 30, 2023
@jezdez jezdez deleted the remove-libmamba-networking branch March 30, 2023 13:11
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Mar 30, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
Archived in project
5 participants