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

PR: Use conda-lock files to incrementally update conda-based installers #22059

Merged
merged 18 commits into from
May 10, 2024

Conversation

mrclary
Copy link
Contributor

@mrclary mrclary commented May 5, 2024

Description of Changes

The update manager plugin now uses an explicitly rendered conda-lock file in order to perform an incremental (minor/micro) update to Spyder in a conda-based installation environment. Previously the update manager simply performed a conda install.

Since conda-forge packages for releases are not available at the time of installer workflow runs, the checksum is removed from the Spyder spec in the conda-lock file. The checksum will be different for the Spyder conda package built on our CI vs. conda-forge, resulting in a ChecksumMismatchError. Removing the checksum in the conda-lock file avoids this error.

@mrclary mrclary self-assigned this May 5, 2024
@pep8speaks
Copy link

pep8speaks commented May 5, 2024

Hello @mrclary! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 337:80: E501 line too long (80 > 79 characters)

Comment last updated at 2024-05-09 20:58:08 UTC

@ccordoba12 ccordoba12 added this to the v6.0beta1 milestone May 5, 2024
@mrclary mrclary force-pushed the conda-lock branch 2 times, most recently from 1feb092 to c9a7406 Compare May 6, 2024 03:10
@mrclary mrclary marked this pull request as ready for review May 6, 2024 04:25
@mrclary mrclary requested a review from ccordoba12 May 6, 2024 04:25
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mrclary for this great improvement! I only left some small questions for you.

installers-conda/build_installers.py Show resolved Hide resolved
installers-conda/build_installers.py Show resolved Hide resolved
spyder/plugins/updatemanager/widgets/update.py Outdated Show resolved Hide resolved
@ccordoba12 ccordoba12 changed the title PR: Use conda-lock files to incrementally update conda-based installations PR: Use conda-lock files to incrementally update conda-based installations (Installers) May 6, 2024
@mrclary mrclary force-pushed the conda-lock branch 2 times, most recently from 45dd879 to ea1b00b Compare May 7, 2024 02:04
@ccordoba12 ccordoba12 changed the title PR: Use conda-lock files to incrementally update conda-based installations (Installers) PR: Use conda-lock files to incrementally update conda-based installers May 7, 2024
@ccordoba12
Copy link
Member

ccordoba12 commented May 7, 2024

Additional question: how are we going to update the conda-lock file from one of our releases to the next? My idea would be to do it like this:

  • For bugfix versions: Take the file from the previous release and only update Spyder and its new direct and optional dependencies on it (i.e. spyder-kernels, pylsp, Pandas, etc). That way we'd be sure that the rest of packages in the installer env would remain exactly the same, which would avoid breakages from transitive dependencies we don't control.
  • For minor and major versions: Update the entire env to include the latest versions of every direct and transitive dependency (or perhaps only a selected group, I'm not really sure). This is more risky, but would also allow us to include important package updates in the installer.
  • Another option would be to be more conservative and only update the entire installer env once or twice per year.

However, with the current approach I think the lock file is auto-generated every time on CIs, which wouldn't allow us to do that.

@mrclary, do you think that workflow is a good idea? If so, how easy do you think it'd be to support it?

@mrclary
Copy link
Contributor Author

mrclary commented May 7, 2024

  • For bugfix versions: Take the file from the previous release and only update Spyder and its new direct and optional dependencies on it (i.e. spyder-kernels, pylsp, Pandas, etc). That way we'd be sure that the rest of packages in the installer env would remain exactly the same, which would avoid breakages from transitive dependencies we don't control.
  • For minor and major versions: Update the entire env to include the latest versions of every direct and transitive dependency (or perhaps only a selected group, I'm not really sure). This is more risky, but would also allow us to include important package updates in the installer.

I think these are good ideas.

  • Another option would be to be more conservative and only update the entire installer env once or twice per year.

I would prefer the previous option, probably because if we update a little more often then fewer things would change and we'd have an easier time identifying any offending packages.

However, with the current approach I think the lock file is auto-generated every time on CIs, which wouldn't allow us to do that.

That is correct.

@mrclary, do you think that workflow is a good idea? If so, how easy do you think it'd be to support it?

I don't think it would be too difficult. conda-lock will "update" a conda-lock file if it already exists, as opposed to over-wrighting it. This could be achieved by either retrieving the conda-lock file from the latest release, or we could version control it in the installers-conda/resources directory. I think we'd just have to consider how we want to handle all the scenarios (PRs, release candidates, and releases).

@ccordoba12
Copy link
Member

I don't think it would be too difficult. conda-lock will "update" a conda-lock file if it already exists, as opposed to over-wrighting it.

Ok, that's really good.

This could be achieved by either retrieving the conda-lock file from the latest release, or we could version control it in the installers-conda/resources directory. I think we'd just have to consider how we want to handle all the scenarios (PRs, release candidates, and releases).

I think it'll be easier to retrieve the file from the latest release and update it on CIs, like this:

  • PRs should update Spyder and the subrepos.
  • Release candidates and releases should only update Spyder.

My rationale is that if we automate this process it'd be one less thing to worry about during the release process. And it'd also be less prone to error.

Could you take care of implementing that (if you agree, of course) in this PR, so we can check the process in beta2?

@mrclary
Copy link
Contributor Author

mrclary commented May 8, 2024

  • PRs should update Spyder and the subrepos.
  • Release candidates and releases should only update Spyder.

👍🏼

My rationale is that if we automate this process it'd be one less thing to worry about during the release process. And it'd also be less prone to error.

Could you take care of implementing that (if you agree, of course) in this PR, so we can check the process in beta2?

Absolutely.

mrclary added 12 commits May 9, 2024 13:57
Note that "local" channel does not work for conda/conda-lock/mamba solver. The local channel path must be explicit. I'm sure that this is an old bug.
This is required by the installer tests.
Fix issue where installer test did not exit if installer failed.
Include scientific packages in lock file.
Ensure PY_VER and SPYVER are updated from spec.
Installer path is either installer or lock file and download worker is used for all updates for conda-based installers.
Shell scripts now "mamba update -p <prefix> -y --file conda-<platform>.lock".
Array options for script and cat-ing to script are incompatible: ("-i" "''" "-e") works for cat-ing but not for script; ("-i" '' "-e") works for script but not cat-ing. For situations that require both script and cat-ing that must work for both GNU and BSD, the only solution is to use "-i.bak -e" and "rm *.bak"
For non-release and non-pre-release, this will be spyder, spyder-kernels, qtconsole, python-lsp-server.
For release and pre-release, this will only be spyder.
@ccordoba12
Copy link
Member

ccordoba12 commented May 10, 2024

@mrclary reviewed the feasibility of my comment above and found out it's not possible to implement it using environment yaml files alone, which will prevent using conda/mamba to perform the update (we'll have to use conda-lock instead).

So, this one is ready.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, thanks @mrclary!

@ccordoba12 ccordoba12 merged commit fdae3f8 into spyder-ide:master May 10, 2024
23 checks passed
@mrclary
Copy link
Contributor Author

mrclary commented May 10, 2024

Thank you @ccordoba12. 😁
Just for full disclosure, the documentation for conda-lock states:

Note: If there is an existing lockfile, it is used as constraint when regenerating the lockfile. This can be useful for adding new packages while keeping everything else locked.

Which does not actually work, at least not the way that I think it does. Furthermore, the documentation shows the example

# optionally, update the previous solution, using the latest version of
# pydantic that is compatible with the source specification
conda-lock --update pydantic

which I could not get to work at all. All of my attempts to use the --update flag resulted in regurgitating the conda-lock cli help; no error or any other messages 🤷‍♂️.

I conclude that either I'm doing something wrong, or there is a bug with conda-lock, or a combination of both. It may be worthwhile to investigate this further in the future.

@mrclary mrclary deleted the conda-lock branch May 10, 2024 19:15
@mrclary mrclary mentioned this pull request May 13, 2024
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants