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: Update Spyder installer base environment with conda-lock file #22300

Merged
merged 16 commits into from
Aug 6, 2024

Conversation

mrclary
Copy link
Contributor

@mrclary mrclary commented Jul 29, 2024

Description of Changes

  • CI now builds a conda-lock file for the installer base environment in addition to the runtime environment.
  • UpdateManager updates the base environment and replaces the runtime environment using conda-lock files.
  • The installer base environment pins all explicit dependencies, including Python.

I've tested the update process for macOS, Linux, and Windows on my local machine and VMs. Both the base and runtime environments appear to update correctly. This is accomplished by installing 6.0.0b3 using the installer, then patching Spyder source code in the runtime environment with changes made in this PR. Additionally I spoof the Spyder version to 6.0.0b2 and change the github url to my fork. base and runtime lock files are created locally and uploaded to a 6.0.0b3.post1 releasse on my fork.

Note that using a lock file for the base environment precludes excluding packages, so pip cannot be excluded.

Note that updating the Python minor version of the base environment results in a broken base environment; only micro versions can be updated with the conda-lock file. When updating the minor version of Python in the base environment, a new installer must be downloaded, therefore updates of Python minor versions to the base environment should be reserved for Spyder major updates.

Because the runtime environment is removed and re-built with the lock file, Python can be updated in this environment with minor/micro Spyder updates and does not require using the installer.

@mrclary mrclary self-assigned this Jul 29, 2024
@pep8speaks
Copy link

pep8speaks commented Jul 29, 2024

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

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

Comment last updated at 2024-08-06 14:54:54 UTC

@mrclary mrclary marked this pull request as ready for review July 29, 2024 16:50
@mrclary mrclary requested review from dalthviz and ccordoba12 July 29, 2024 16:50
@mrclary mrclary added this to the v6.0rc1 milestone Jul 29, 2024
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 your work on this!

installers-conda/build-environment.yml Outdated Show resolved Hide resolved
spyder/plugins/updatemanager/scripts/install.sh Outdated Show resolved Hide resolved
spyder/plugins/updatemanager/workers.py Outdated Show resolved Hide resolved
@ccordoba12
Copy link
Member

@mrclary, one more question for you: is it possible to zip all lock files into a single file and upload only that artifact to the GH release?

I say that because Github displays all artifacts for the latest release, and I think having too many lock files would be distracting for users.

@mrclary
Copy link
Contributor Author

mrclary commented Jul 31, 2024

@mrclary, one more question for you: is it possible to zip all lock files into a single file and upload only that artifact to the GH release?

Perhaps this can be done...

If the installer jobs only upload the artifacts (but not to the release), perhaps a separate job after the installer jobs can retrieve all the artifacts, zip all the lock files, and upload all the artifacts to the release. This would require minor changes to the installer workflow, and some changes to the update manager.

I'll look into it.

@mrclary mrclary force-pushed the conda-lock-base branch 5 times, most recently from 67c0e26 to 4f8db75 Compare August 1, 2024 17:59
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.

A couple of minor questions above the commands used to unzip the new file that contains all lock files.

The rest looks good to me.

spyder/plugins/updatemanager/scripts/install.bat Outdated Show resolved Hide resolved
spyder/plugins/updatemanager/scripts/install.sh Outdated Show resolved Hide resolved
@mrclary
Copy link
Contributor Author

mrclary commented Aug 2, 2024

Okay, the osx-arm64 lock file now has asyncssh=2.14.1 and I've successfully tested the update process on all 4 platforms.

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.

Looks good to me now, thanks @mrclary!

@ccordoba12
Copy link
Member

About your notes:

Note that using a lock file for the base environment precludes excluding packages, so pip cannot be excluded.

Ok, but is it possible for users to select the base env either in Preferences > Main interpretrer or through the Consoles > New console in environment menu?

If so, we need to prevent users to do that before 6.0 so that they can't install packages in that env,

Note that updating the Python minor version of the base environment results in a broken base environment; only micro versions can be updated with the conda-lock file. When updating the minor version of Python in the base environment, a new installer must be downloaded, therefore updates of Python minor versions should be reserved for Spyder major updates.

I think that's too restrictive. Instead, we could update Python every two minor Spyder versions (i.e. we should update to Python 3.12 in Spyder 6.2). But we can talk about that in another issue.

@mrclary
Copy link
Contributor Author

mrclary commented Aug 2, 2024

Note that using a lock file for the base environment precludes excluding packages, so pip cannot be excluded.

Ok, but is it possible for users to select the base env either in Preferences > Main interpretrer or through the Consoles > New console in environment menu?

No, users cannot select the base environment through either of these methods. The installer explicitly prevents the base and runtime environments from being registered, so they don't show up in conda info -e and Spyder does not list them in the possible interpreters. The runtime environment is only available as "Same as Spyder" interpreter selection.

@mrclary
Copy link
Contributor Author

mrclary commented Aug 2, 2024

Note that updating the Python minor version of the base environment results in a broken base environment; only micro versions can be updated with the conda-lock file. When updating the minor version of Python in the base environment, a new installer must be downloaded, therefore updates of Python minor versions should be reserved for Spyder major updates.

I think that's too restrictive. Instead, we could update Python every two minor Spyder versions (i.e. we should update to Python 3.12 in Spyder 6.2). But we can talk about that in another issue.

So while investigating this, I've discovered another issue. When updating the runtime environment with the lock file, obsolete packages are not removed. Updated packages are correctly replaced and new packages are added, but if a package in the old environment is no longer needed, it is not removed. This is not good.

I think the best solution is to remove the runtime environment and recreate it with the lock file, rather than try to update it. This will also solve the issue of updating Python in the runtime environment, i.e. we could update Spyder to use Python 3.12 at any time.

However, there could still be a potential issue updating the base environment. Here, we cannot remove and recreate the environment. The base environment does not need to have the same Python version as Spyder's runtime, so we could still refrain from updating Python in the base environment until major updates to Spyder. So the only issue is whether the base environment update will ever require removing unnecessary packages. This may be an extremely low risk since there isn't an extensive list of packages in the base environment (only Python, conda, mamba, and menuinst are specified).

What are your thoughts, @ccordoba12?

@dalthviz dalthviz mentioned this pull request Aug 2, 2024
@mrclary mrclary force-pushed the conda-lock-base branch 2 times, most recently from 391f822 to 15b6f9d Compare August 3, 2024 21:12
@ccordoba12
Copy link
Member

So while investigating this, I've discovered another issue. When updating the runtime environment with the lock file, obsolete packages are not removed.
I think the best solution is to remove the runtime environment and recreate it with the lock file, rather than try to update it. This will also solve the issue of updating Python in the runtime environment, i.e. we could update Spyder to use Python 3.12 at any time.

I saw that you already implemented this, but I kind of disagree with doing it for every release. That's because it defeats the purpose of the update process, which is to have a simple and fast update. The fact that obsolete packages are not removed is (in my opinion) a minor nuisance that we can deal with every minor (or two minor releases).

In other words, I wouldn't like users to download close to 400 mb of packages every time a new version is available. That's almost the same as downloading the full installer.

@mrclary
Copy link
Contributor Author

mrclary commented Aug 5, 2024

TLDR:

Suggested changes:

  • Let's update the runtime environment for micro updates and replace the runtime environment on minor updates. Installer is still required for major updates.
  • Let's retain the package cache in the base environment in order to minimize required downloads in subsequent updates. We should still run a conda clean --packages in the post install script to save disk space.

Details

I saw that you already implemented this,

I can revert, but wanted to test an implementation in order to have it ready as soon as possible should we want it.

but I kind of disagree with doing it for every release. That's because it defeats the purpose of the update process, which is to have a simple and fast update. The fact that obsolete packages are not removed is (in my opinion) a minor nuisance that we can deal with every minor (or two minor releases).

I agree that this is not ideal. Replacing the runtime environment is the only way to update the Python version outside of a major release. Perhaps the update manager can also determine whether the update is a minor update or a micro (bug fix) update? Then it could instruct the install script to either update the runtime environment for micro updates or remove and re-create the environment for minor updates and we could ensure that Python is only updated on minor updates. The update manager will still use a new installer for major updates.

In other words, I wouldn't like users to download close to 400 mb of packages every time a new version is available. That's almost the same as downloading the full installer.

Conda will download every package listed in the lock file if it does not already exist in the pkgs cache directory of the base environment. Currently, we have the installer clear the pkgs directory after the install is complete in order to save disk space, so the first minor or micro update will download all the packages anyway, regardless whether we update the runtime environment or replace it. Subsequent updates will only download missing packages. We could instruct the installer to leave the pkgs directory so even the first update will download fewer packages.

For what it's worth, in my testing this PR, downloading and installing with the installer takes much longer than the install script to download all packages and replace the environment, though the latter does take longer than just an update process.

@ccordoba12
Copy link
Member

ccordoba12 commented Aug 6, 2024

Let's update the runtime environment for micro updates and replace the runtime environment on minor updates. Installer is still required for major updates.

What about doing the update every two minor versions? That would make our installers really stable and I think updating to a new Python version once per year is fine (I mean, it'd follow more or less the Python release cycle, but we'd always be one version behind the latest one for stability).

Let's retain the package cache in the base environment in order to minimize required downloads in subsequent updates. We should still run a conda clean --packages in the post install script to save disk space.

That could help to address my main concern but Conda-forge moves way too fast, so I don't know how useful this could be. In addition, since you said that updating takes less time than actually downloading the installer (probably because downloading Conda packages is done in parallel), then we should leave things as they are (i.e. cleaning the cache after the installation ends).

I can revert, but wanted to test an implementation in order to have it ready as soon as possible should we want it.

That was good a idea but since it's not related to this PR, and we want to release rc1 in two or three days, I think you should revert it and try it in another one.

Perhaps the update manager can also determine whether the update is a minor update or a micro (bug fix) update? Then it could instruct the install script to either update the runtime environment for micro updates or remove and re-create the environment for minor updates and we could ensure that Python is only updated on minor updates.

That's a good idea.

@ccordoba12
Copy link
Member

ccordoba12 commented Aug 6, 2024

Since @dalthviz is mostly unavailable this week, I'm going to merge this one because we need it for rc1.

Thanks again @mrclary for your hard work on our installers!

@ccordoba12 ccordoba12 merged commit 5140f45 into spyder-ide:master Aug 6, 2024
24 checks passed
@mrclary mrclary deleted the conda-lock-base branch August 6, 2024 18:27
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