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

Report progress in validating downloads #3659

Merged
merged 2 commits into from
Sep 23, 2022

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Sep 7, 2022

Motivation

  • After a mod finishes downloading and before we install it, we check that it's a valid ZIP file and that it matches the hashes in that module's metadata. This takes a noticeable amount of time for mods larger than about ~30MB (several seconds), which frighteningly probably scales linearly to the GB+ mods that we index, and there is no indication of what's happening; it looks like CKAN has frozen. As part of the general improvements for downloading in the next release, I want to increase the transparency of this process.
  • The per-module progress bars in the install flow can be overlapped by their labels when a module name is long
  • Per-module progress bars aren't shown when downloading a mod to cache without installing, even though you can download multiple in parallel by jumping back to the mod list and starting more

Changes

  • Now a new CKAN.Extensions.CryptoExtensions.ComputeHash extension function supports progress updates from 0 to 100 via an IProgress parameter and cancellation via a cancellation token (cancellation not used yet but could be taken advantage of in the future)
  • Now NetFileCache.ZipValid, NetFileCache.GetFileHashSha1, NetFileCache.GetFileHashSha256, and NetModuleCache.Store also have an IProgress parameter for reporting progress from 0 to 100 in each of their respective tasks. NetModuleCache.Store scales and combines the updates from its sub-tasks in a 60:20:20 ratio for ZipValid:GetFileHashSha1:GetFileHashSha256, since that seems to be about how long they take in practice.
    • The nearly identical NetFileCache.GetFileHashSha1 and NetFileCache.GetFileHashSha256 are mostly factored out into a new generic function NetFileCache.GetFileHash<T> that they call
  • Now the Wait tab uses a TableLayoutPanel to manage the per-module progress bars
    • The labels and progress bars always fit neatly into 2 columns without overlapping
    • The code is simpler and no longer micro-manages locations or sizes
  • Now IDownloader has a new StoreProgress event for reporting progress as it validates a ZIP file
  • NetAsyncModulesDownloader fires its StoreProgress event in response to progress updates from NetModuleCache.Store
  • Now the install and download to cache flows use the new StoreProgress event to show a per-module progress bar while downloads are being validated so the user can see what's happening
    • Download to cache also shows regular per-module download progress bars
    • We also print one additional message to the log when we have finished downloading and are starting validation of a download, so users can see what's happening there as well (and for CmdLine)

Additional fixes

With the major features of my ongoing network handling revamp completed, but mainly developed on Windows, I decided to take the master branch for a spin in an Ubuntu VM. I found myself on a very lively bug hunt, but understanding what was happening in various confusing cases required this PR's progress bars for ZIP validation, so I rebased and added the fixes here:

  • The failed downloads dialog from Many improvements for failed downloads #3635 was opened on a background thread instead of the UI thread, which completely crashed it on Mono (but worked on Windows). Now it opens on the UI thread as it should, and a TaskCompletionSource tells us when it's done, as in several existing spots.
    • The failed downloads dialog's auto-sizing wasn't tall enough on Mono, now it includes the grid headers and padding
    • Improvements for failed repo updates #3645 renamed the DownloadRow.Module property to Data, but the grid was still looking for Module. Now it's updated to Data.
  • You can cancel downloads, and they really cancel, leaving behind the partial file as per Resume failed downloads #3666
    • The async downloader no longer tries to cancel downloads that already finished or failed
    • The async downloader no longer starts queued downloads after cancellation
  • If ResumingWebClient finds that it already has the entire file in the in progress folder, it closes the stream, but Mono's WebClient seems to re-open it (whereas Windows leaves it closed), so now we also treat a download as unnecessary if contentLength is 0
  • The checkbox above the mod list that can uninstall all mods or clear the changeset now operates much quicker by updating the changeset once at the end instead of for every row
  • Install dependencies first #3667 broke mod upgrades because the ModUpgrade class's constructor no longer handled the parameters GUIMod gave it correctly, now this is fixed
  • Wait.SetProgress will no longer divide by 0 if you pass 0 in its total param
  • ManageMods.ModGrid was renamed from ModList in Master search bar and misc GUI clean-up #3041, but its event handlers still had a ModList_ prefix; now they finally start with ModGrid_
  • Unreachable code is removed from Repo.ModulesFromFile
  • Kraken classes that used \r\n now use Environment.NewLine instead to match the host platform
    • Various public fields in the Kraken classes are now marked readonly where possible
  • Several debug messages are shortened from printing whole exceptions to just Exception.Message, to make them easier to browse

@HebaruSan HebaruSan added Bug Something is not working as intended Enhancement New features or functionality GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN Pull request Network Issues affecting internet connections of CKAN labels Sep 7, 2022
@HebaruSan HebaruSan force-pushed the feature/store-progress branch from dc7468d to d771a37 Compare September 13, 2022 21:53
@HebaruSan HebaruSan added the In progress We're still working on this label Sep 13, 2022
@HebaruSan

This comment was marked as resolved.

@HebaruSan HebaruSan force-pushed the feature/store-progress branch 3 times, most recently from 5fdf546 to 6e540da Compare September 15, 2022 01:50
@HebaruSan HebaruSan removed the In progress We're still working on this label Sep 15, 2022
@HebaruSan
Copy link
Member Author

OK, this should now be ready! And it fixes some pretty serious problems.

@HebaruSan HebaruSan force-pushed the feature/store-progress branch from 6e540da to 9757ade Compare September 22, 2022 14:11
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

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

That looks like a significant Quality of Life improvement! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality GUI Issues affecting the interactive GUI Network Issues affecting internet connections of CKAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants