[update] Allow downgrades during nextstrain update conda
#266
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tsibley
changed the title
nextstrain update conda
|
Hmm. This worked for my fresh reproduction of the issue in #264 in another |
|
Ok, it's running into some other conflict that it can't/won't resolve under the current conditions, and so "solves" the update request as a no-op. To resolve this in the way we want, it needs to uninstall Two thoughts:
|
|
It looks like (2) will work. |
|
Why not ask it to install the most recent version directly rather than any version that's later than current? IIUC, mamba/conda keeps packages around even after uninstall, so an uninstall, reinstall should be almost as cheap as update. But maybe this doesn't apply to micromamba. |
That's what we do for our Docker runtime, but I avoided it for Conda because it requires determining the latest version first and that can potentially be problematic to do correctly given that the way packages are platform-specific (differently than Docker's are). It seemed easier to just let Micromamba handle it. But I guess doing it ourselves ends up being about the same as what the Conda runner already does to download Micromamba when setting up the runtime. So yeah, maybe I'll do that.
It does apply to Micromamba, but even without redownload, reinstall won't be as cheap as update due to the unlink/relinking cycle. It's also nice to show the diff of versions on update. |
To do this (or also my previous inclination) properly required a bit more change than it seemed at first, but I have a nice and robust solution now. Need to clean it up a bit and push additional commits to this PR, but will save that for next week. |
This permits Micromamba to downgrade packages that are already installed in order to upgrade nextstrain-base. While nextstrain-base's fully-locked dependencies from version to version will typically only cause upgrades, there are times when downgrades may be required.¹ When that's the case, Micromamba simply won't consider that nextstrain-base version as available without --allow-downgrade. Resolves <#264>. ¹ For example, the deps of Nextstrain CLI's package in Bioconda changed to better match its deps in PyPI², which meant versions of botocore, boto3, and some related packages had to be downgraded. ² <bioconda/bioconda-recipes#39711>
Not just after setup. This ensures we remove older versions of now-upgraded packages so they don't keep taking up disk space unnecessarily.
…age distribution I'm about to want to do the same query for our nextstrain-base package to facilitate better update behaviour.
…versions Consistent with the rest of the runner, which uses a package name overridable by an environment variable.
…ONDA_BASE_PACKAGE Previously you could smuggle a package match spec (e.g. ==X) into NEXTSTRAIN_CONDA_BASE_PACKAGE because setup() and update() simply passed it thru to Micromamba. We even took advantage of this in the testing instructions produced by our conda-base CI.¹ As I started to add more logic in update() and not just pass thru the package name, I realized this version smuggling use case would pose an issue and the code needed the values to be more predictable. I didn't want to remove the ability to specify a version spec though, nor parse the ambiguous and (seemingly) not fully-specified `conda install`-style specs, so I settled on properly supporting the more formal two- and three-part Conda package match specs.² This commit plumbs support for those thru the places which previously assumed to take just a package name. Note that this env var is still considered for development and testing only, so this change shouldn't have any user-visible impact. ¹ <https://github.com/nextstrain/conda-base/blob/b8b9ca82/.github/workflows/ci.release.md> ² <https://docs.conda.io/projects/conda/en/latest/user-guide/concepts/pkg-specs.html#package-match-specifications>
This helps Micromamba (or really, libsolv) take the action we want, upgrading to the latest version, as otherwise it sometimes chooses a solution that results in no upgrade.¹ It's also good to be explicit about the version we expect, and this behaviour is closer to what we do when updating other runtimes as well. The update messaging is also improved in the no-op case by having the desired version on hand. ¹ e.g. <#266 (comment)>
tsibley
force-pushed
the
trs/conda/update/allow-downgrades
branch
from
00757de to
d6e4f2b
Compare
|
Repushed with additional fixes. |
|
If folks want to test this locally, one way is to use the standalone build for this PR: |
tsibley
added a commit
to nextstrain/conda-base
that referenced
this pull request
Mar 21, 2023
…A_BASE_PACKAGE Per <nextstrain/cli#266>. Applying in advance of that Nextstrain CLI change, which should be safe as both package specs should be accepted by Micromamba.
victorlin
approved these changes
Mar 24, 2023
There was a problem hiding this comment.
Skimmed through the changes and tested locally:
-
My current Conda runtime version is
20221019T172207Z.% nextstrain version --verbose | grep -F "nextstrain-base" Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running? nextstrain-base 20221019T172207Z (h0dc7051_1_locked, nextstrain) -
The latest available version is
20230314T174809Z.% CONDA_SUBDIR=osx-64 conda search --override-channels -c nextstrain nextstrain-base | tail -n 1 nextstrain-base 20230314T174809Z h0dc7051_1_locked nextstrain -
CLI 6.2.0 updates to
20230303T182321Z.% nextstrain version nextstrain.cli 6.2.0 % nextstrain update conda - nextstrain-base 20221019T172207Z h0dc7051_1_locked nextstrain 9kB + nextstrain-base 20230303T182321Z h0dc7051_1_locked nextstrain/osx-64 9kB -
CLI from this PR updates to
20230314T174809Z.% /tmp/nextstrain-cli/nextstrain version nextstrain.cli 6.2.0+git.f3b209b % /tmp/nextstrain-cli/nextstrain update conda - nextstrain-base 20230303T182321Z h0dc7051_1_locked nextstrain Cached + nextstrain-base 20230314T174809Z h0dc7051_1_locked nextstrain/osx-64 9kB
|
@victorlin Awesome, that's exactly the sort of confirmation I was hoping to see from others. 🙌 Thanks! 🙏 |
|
Can confirm this works in production :)
|
|
It is weird though that we suggest to install nextstrain-cli 6.2.0 🙃 |
Ha. A little, yeah. It'd be possible to mount a standalone installation of Nextstrain CLI into the runtime so the exact same version outside the runtime vs. inside the runtime is used. But for other installation methods it wouldn't be. So it seems easiest/simplest/best to not try to use the same installation inside and outside the runtime. The point of having it in the runtime is mostly for simple invocations of the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commit messages.
Related issue(s)
#264
Testing
nextstrain update condadoesn't seem to update to latest available conda-base #264