Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Use indicatif to provide a more intuitive representation of upload progress #956

Merged
merged 7 commits into from
Dec 17, 2019

Conversation

gabbifish
Copy link
Contributor

@gabbifish gabbifish commented Dec 13, 2019

This PR closes #906. It uses the indicatif crate to represent progress in KV uploads for workers sites. This provides more context to upload progress than Uploading... messages.

Now uploads look like

$ wrangler publish
 Using namespace for Workers Site "__blog-workers_sites_assets"
 Uploading site files
████████████████████████████████████████████████████████████████████████ 48/48
 Success
⬇️ Installing wasm-pack...
 Built successfully, built project size is 11 KiB.
 Successfully published your script to https://blog.ziggy.workers.dev

Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper left a comment

Choose a reason for hiding this comment

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

Hmm... On a newly generated workers site I get

$ wrangler publish
🌀  Using namespace for Workers Site "__worker-workers_sites_assets"
🌀  Uploading site files
██████████████████████████████████████████████████████████████████████████████████ 0/0
✨  Success
added 2 packages from 2 contributors and audited 2 packages in 0.375s
✨  Built successfully, built project size is 11 KiB.
✨  Successfully published your script to https://worker.avery.workers.dev

On larger sites I can get it to say 44/44 but it doesn't seem to do any waiting, it just prints out a full progress bar.

@gabbifish
Copy link
Contributor Author

It looks like you were using a pre-existing namespace with all the site files already uploaded (make sure to delete existing namespaces if you're testing this functionality!)

As for the full progress bar, this is because Wrangler already uploads up to 5000 files (or max 50MB files) in one go. You're uploading a tiny site, it seems, so the upload happens in one call. Indicatif will be more useful for our users uploading way more substantial sites.

@gabbifish
Copy link
Contributor Author

Something I can do, though, is not use the indicatif bar if the number of pairs being uploaded == 0. I'll get on that.

@EverlastingBugstopper
Copy link
Contributor

I'm wondering if it's better to not display the bar for sites under 5k files. It seems strange to have a progress bar that's always entirely filled. Definitely think it's useful for larger sites though. Will it jump in increments of 5000 because even that might look a little weird.. interested in other opinions about this!

@gabbifish
Copy link
Contributor Author

gabbifish commented Dec 16, 2019

I like the idea of not displaying the bar for small uploads. This is more preferable than breaking up the API calls into unnecessarily small batches for the sake of a nice looking indicatif bar.

And yes, the bar will jump in batches of 5000 (or whatever files amount to the 50MB payload max for uploads). I don't think that would look weird, given that it would be an accurate representation of how many files are uploaded.

@EverlastingBugstopper
Copy link
Contributor

I wonder if it would be worth doing just like a little tiny 1ms sleep for each step in the 5000 or something... /how hard that would be

@gabbifish
Copy link
Contributor Author

That assumes that there's a certain duration for the API call, which is not the case. I'm strongly in favor of displaying accurate information to Wrangler users, even if it's not "pretty". I'd rather peg indicatif monitors to accurate information.

Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use indicatif for output
3 participants