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

Simplify reconstruction of fetch actions #376

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

maresb
Copy link
Contributor

@maresb maresb commented Feb 28, 2023

I'm very interested to see if this has any affect on #338.

@netlify
Copy link

netlify bot commented Feb 28, 2023

Deploy Preview for conda-lock ready!

Name Link
🔨 Latest commit b39bea4
🔍 Latest deploy log https://app.netlify.com/sites/conda-lock/deploys/6400b76083979a0008d8c0fa
😎 Deploy Preview https://deploy-preview-376--conda-lock.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@maresb
Copy link
Contributor Author

maresb commented Feb 28, 2023

pre-commit.ci autofix

@mariusvniekerk
Copy link
Collaborator

Seems like this does improve the md5 test issue?

@maresb
Copy link
Contributor Author

maresb commented Mar 2, 2023

☝️ squash

@mariusvniekerk, yes, this resolves it. In the process I noticed a whole bunch of illogical stuff regarding the formation of LockedDependencies which I wanted to clean up, but it goes deeper than I originally thought. I think it makes sense to merge this so that CI works, and follow up with the remaining stuff in future PRs.

👇 rebase

@maresb maresb marked this pull request as ready for review March 2, 2023 14:49
@maresb maresb requested a review from a team as a code owner March 2, 2023 14:49
@maresb
Copy link
Contributor Author

maresb commented Mar 2, 2023

As a brief explanation, my prevailing theory (based on substantial evidence) is that when Micromamba computes the dry-run LINK info, it tries to grab the hashes from the repodata_record.json with an internal implementation similar to #373, but without the retry. Thus in the rare case when repodata_record.json has not been extracted completely, Micromamba leaves the hashes blank.

This PR abandons the approach of filling in missing Micromamba FETCH actions with their corresponding LINK actions, instead using the more robust repodata_record.json reader.

@maresb
Copy link
Contributor Author

maresb commented Mar 2, 2023

@mariusvniekerk, all tests are green, so if you approve, then we can merge this, fixing most of the CI issues.

As a follow-up to this, we should make sure that the approaches in update_specs_for_arch and solve_specs_for_arch are as similar as possible, and eliminate duplicated code. Moreover, since the data structures for LinkAction are different between Micromamba and Conda/Mamba, we should redefine LinkAction to Union[MicromambaLinkAction, CondaLinkAction] with the respective models. This lights up the type checker, revealing some substantial issues, which is why I think it deserves its own PR.

@mariusvniekerk mariusvniekerk merged commit cdca3e4 into conda:main Mar 3, 2023
@mariusvniekerk
Copy link
Collaborator

cc @wolfv there is probably some stuff in this discussion that you should be aware of

@maresb maresb deleted the simplify-fetch-actions branch March 3, 2023 06:51
@wolfv
Copy link

wolfv commented Mar 3, 2023

@maresb can you share a bit more of the symptoms? What I understand is that the repodata_record.json contains the correct complete values for MD5 and SHA256, but micromamba is somehow not reading them?

@maresb
Copy link
Contributor Author

maresb commented Mar 3, 2023

Ya, I will try to open a proper issue soon in mamba-org/mamba with more details. It relates to some apparent race condition that occurs on a regular basis on our test suite but is not reliably reproducible.

We have multiple parallelized tests spawning Conda/mamba/micromamba, but I am not sure whether or not this is a multiprocess problem.

The problem seems to occur when we do a dry-run solve to get LINK and FETCH action JSON data, and the md5 and sha256 fields of a link-only package are missing from this output. When this triggers, I immediately look (programmatically) at the installation location of the package in question, and it looks like extraction is incomplete. It completes after sleeping for 100 ms, and if I then repeat the dry run it is successful.

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.

None yet

3 participants