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

Show Download Progress #609

Merged
merged 16 commits into from
Apr 7, 2022
Merged

Show Download Progress #609

merged 16 commits into from
Apr 7, 2022

Conversation

shundhammer
Copy link
Contributor

@shundhammer shundhammer commented Mar 24, 2022

Target Branch / Product

This is for SLE-15-SP4.

Version for master: #612

Bugzilla

https://bugzilla.suse.com/show_bug.cgi?id=1195608

Trello

https://trello.com/c/38oLzang/

Problem

During package installation (only in the installed system, not during installation) there was no visual feedback while packages were being downloaded, only when all downloads were finished and it started actually installing the downloaded packages: The user saw only a progress bar stuck at 0% for minutes while the download was in progress.

During system installation, this was not a problem since download and installation are now (since SLE-15 SP4 / Leap 15.4) done in parallel.

Cause

After the progress reporting was greatly simplified to enable parallel actions in libzypp, there was only one single progress bar, no lists of current actions (which had included downloads).

Fix

Now displaying both the downloads and the installed / upgraded / removed packages in that single progress bar.

  • Total: Expected (unpacked) install sizes of all packages + expected (packed) download sizes

  • Current: Finished install sizes + finished download sizes + partial download sizes

Video

no-popup.mp4

Rationale

Of course this is a little apples vs. oranges: Downloading 100 MB of RPMs rarely takes the same time as installing 100 MB of RPMs. But this makes the progress bar move during all waiting times, so the user can see that there is progress.

While doing similar operations on the command line, many seasoned users do

sudo zypper refresh
sudo zypper dup --download-only -y

And only when this is finished:

sudo zypper dup

This makes sure that this is a transaction, and it doesn't get stuck in the middle because of some packages that could not be downloaded. The current policy of the YaST software module in the installed system does very much the same: First, all required packages are downloaded, and when that is finished, the actual install / upgrade / remove operations are started.

Depending on the user's Internet connection, the download phase may take considerably longer than the installation phase, so the progress bar will very likely move quicker during the second phase. But it is very unlikely that any user will complain when it speeds up during the process; that will be very welcome by most users.

Parallel Download and Installation

During system installation, we are now using a new libzypp mode that does downloading and installing packages in parallel, so the code needs to take that into account.

The first naive approach was to just check for Mode.normal (i.e. only in the installed system, not during system installation / system upgrade / AutoYaST installation). That works right now, but who knows when we will start using that new mode also in other scenarios.

So the code now keeps track in the callbacks (DownloadStart(), DownloadProgress(), DownloadEnd(), PkgInstallStart(), ...) if there is any indication that a package starts being installed while another one is downloaded, and changes to the parallel_download mode, i.e. the progress bar only takes the package installation into account and not the downloads anymore; i.e. it auto-adapts.

The initial default is still based on Mode.normal, though.

Other Changes

  • Cleaned up that PackageSlideShow module:
    • Removed code duplication (initializing the member variables)
    • Removed redundant member variables: Some counter variables were really only the size of lists (packages installed, packages updated, ...). It never made any sense to duplicate that information. Now using @xxx_list.size instead wherever needed.
    • Clearly renamed the download callbacks to indicate what they do.
    • Factored out the very uncommon DownloadError() case into a separate method for clearer control flow; the uncommon case should not obscure the common one (download finished normally), much less with always passing 2 (!) out of 3 method parameters for the uncommon error case.
    • Removed most YCP zombies (Ops.Add(), seriously?!)

Installer Self-Update?

This affects only the installed system, not the system installation or upgrade, so an installer self-update is not needed.

Related PR

@coveralls
Copy link

coveralls commented Mar 24, 2022

Coverage Status

Coverage decreased (-0.02%) to 36.811% when pulling 82d8044 on huha-dl-progress into 78488dd on master.

@shundhammer shundhammer force-pushed the huha-dl-progress branch 3 times, most recently from cb39332 to d67203d Compare March 28, 2022 15:41
src/modules/PackageSlideShow.rb Show resolved Hide resolved
src/modules/PackageSlideShow.rb Outdated Show resolved Hide resolved
src/modules/PackageSlideShow.rb Show resolved Hide resolved
src/modules/PackageSlideShow.rb Show resolved Hide resolved
src/modules/PackageSlideShow.rb Show resolved Hide resolved
@shundhammer shundhammer changed the base branch from master to SLE-15-SP4 April 6, 2022 13:49
@shundhammer shundhammer marked this pull request as ready for review April 6, 2022 14:07
Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

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

LGTM

@shundhammer shundhammer merged commit 0ecb901 into SLE-15-SP4 Apr 7, 2022
@shundhammer shundhammer deleted the huha-dl-progress branch April 7, 2022 10:48
@yast-bot
Copy link
Contributor

yast-bot commented Apr 7, 2022

✔️ Internal Jenkins job #4 successfully finished
✔️ Created IBS submit request #269218

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

4 participants