-
Notifications
You must be signed in to change notification settings - Fork 2k
Allow repodata caching for local channels #9677
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
Allow repodata caching for local channels #9677
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have one on file for @LorcanHamill. In order for us to review and merge your code, please e-sign the PDF at https://conda.io/en/latest/contributing.html#conda-contributor-license-agreement. We then need to manually verify your signature. We will ping the bot to refresh the PR status when we have confirmed your signature. |
|
Sorry, unit tests are failing - I'll investigate and push a fix as soon as possible. |
|
We require contributors to sign our Contributor License Agreement, and we don't have one on file for @LorcanHamill. In order for us to review and merge your code, please e-sign the PDF at https://conda.io/en/latest/contributing.html#conda-contributor-license-agreement. We then need to manually verify your signature. We will ping the bot to refresh the PR status when we have confirmed your signature. |
1 similar comment
|
We require contributors to sign our Contributor License Agreement, and we don't have one on file for @LorcanHamill. In order for us to review and merge your code, please e-sign the PDF at https://conda.io/en/latest/contributing.html#conda-contributor-license-agreement. We then need to manually verify your signature. We will ping the bot to refresh the PR status when we have confirmed your signature. |
|
This is failing still in a number of ways. I've now found out how to run the integration tests offline, so I'll see if I can fix them in a subsequent submit. I'll be on vacation for a few days, though, so that's unlikely to happen until next week. Should I close this PR and open a new one later? Or is it okay to just leave it here until I get to fix it? Also, I keep getting told by the bot that there is no CLA on file for me. I've signed it, twice. Is that just awaiting some manual verification? |
|
This is huge - applied the fix and it worked like a charm on my end. Thanks! |
|
Maybe have an optional parameter to enable this functionality so it is disabled by default (can be enabled in condarc for example)? That is a "roundabout" way to pass the unit tests, though I dont think that is proper. I do feel that this is a useful feature to have, even if there is a disclaimer stating it isnt robustly tested yet. |
|
@msarahan @kalefranz Could this please get merged in if possible? (a patch release with this fix would be great too as it's a major regression) With multiple file-based channels, a single conda install command may currently take up to 5 minutes (and cause hundreds of mb of traffic) due to the same repositories being queried hundreds of time, over and over. |
|
Apologies for the delay on this. I'll try to get a fix today for the failing tests. Also, what do I have to do about the CLA error? I have signed the CLA - twice as I recall!? |
|
Thanks for the PR. Making inquiries about the CLA. |
|
We require contributors to sign our Contributor License Agreement, and we don't have one on file for @LorcanHamill. In order for us to review and merge your code, please e-sign the PDF at https://conda.io/en/latest/contributing.html#conda-contributor-license-agreement. We then need to manually verify your signature. We will ping the bot to refresh the PR status when we have confirmed your signature. |
21baf27 to
e9a5056
Compare
|
I closed this PR accidentally by resetting the branch and force-pushing it, while trying to work out why tests were failing. I've now submitted a new PR - #9730 . That just adds a new method that is never called, yet lots of tests are broken. Any insight on that would be appreciated. |
|
Hi there, thank you for your contribution to Conda! This pull request has been automatically locked since it has not had recent activity after it was closed. Please open a new issue or pull request if needed. |
As discussed in issue #9661 the performance of "file:///" channels is very poor in some circumstances with the latest conda versions. This seems to be because the solver requests repodata about directories more often than it used to. There is a caching mechanism for repodata but it is explicitly disabled for local channels; this PR removes that check so that the caching mechanism works for file:/// channels in the same way as HTTP channels.
That broke some unit tests, because of the way that an environment variable can be set to force conda to use only .tar.bz2 files, not .conda files. The tests try both settings, which changes the repodata that is fetched and should therefore be cached. The tests fail because the wrong version of the data appears in the cache. This could never happen in production as the cache is in memory. So it starts out empty when the conda command is invoked, and the command will only see one setting of the environment variable, so the cache will only ever have the appropriate data. To fix the tests, I added a method to clear the cache and invoked it before each test.
This change makes a huge difference to my company's use of conda. We use large conda channels on relatively slow NFS volumes, and without this change the creation of a new environment takes minutes when it previous took a few seconds.
This is my first PR on conda - please let me know if I have done anything wrong... 😄
Fixes #9661