Add progress-bar to common/step_download.go #5851

arizvisa · 2018-02-05T21:53:43Z

This was based on PR #2906 and so had to be delayed until that was merged upstream.

This adds a progress-bar to step_download.go using the progressbar implementation at http://github.com/cheggaaa/pb that was suggested on an earlier issue.

The progress-bar's appearance was consolidated into a function so that it can be changed to look however you guys want. It's currently been specified to provided the same information as the previous download ticker as well as including the number of b/mb/gb that have been xferred.

This also removes the previous PercentProgress ticker as it's been superceded by this progressbar.

Closes #6068

arizvisa · 2018-02-14T17:53:43Z

Re-based onto master.

arizvisa · 2018-02-14T18:17:13Z

Created issue #5903 for this PR.

arizvisa · 2018-06-14T22:36:09Z

Travis-CI failed when running tests against go 1.9.x due to a connection error and thus this might need to be resetted (package golang.org/x/tools/cmd/stringer: unrecognized import path "golang.org/x/tools/cmd/stringer" (https fetch: Get https://golang.org/x/tools/cmd/stringer?go-get=1: net/http: TLS handshake timeout)).

Is there still interest in this PR? If so I can rebase to fix the conflict.

SwampDragons · 2018-06-15T16:15:21Z

For now let's leave it open but not spend time fixing it up -- I'm still chugging along through replacing our download code and don't want you to duplicate work. Sorry it's taking so long.

arizvisa · 2018-06-15T21:44:44Z

kk. no worries.

SwampDragons · 2018-07-23T19:40:33Z

You know what? The go-getter replacement is taking me forever. If you want to rebase this, I'll merge it for the 1.3 release.

arizvisa · 2018-07-24T19:23:35Z

ok, sure.

arizvisa · 2018-07-24T20:16:40Z

kk. it's been rebased. I haven't tested this in a while (and won't be able to as we're approaching the end of a quarter), but nothing significant has really changed so the only difference that you should see is the packer progress-bar being replaced with cheggaaa's progress-bar.

if you want to customize the way the progress-bar looks, there's a function in common/step_download.go called GetDefaultProgressBar() that sets the default appearance.

again, this progress-bar only works for downloading stuff as it's based on the work I did earlier converting the downloaders to use goroutines. if a similar interface comes out for uploading across the different builders, I can look at integrating a progress-bar there too.

arizvisa · 2018-07-24T20:18:45Z

In reference to the delays with go-getter, should I re-open PR #5849 (and rebase it) for FTP support?

SwampDragons · 2018-07-24T23:09:49Z

Let's leave the FTP support one alone for now (reason being, if I DO implement go-getter, that'd be a regression since I don't believe go-getter supports FTP either.) . I'll test this PR soon and merge it when 1.3 rolls closer.

SwampDragons · 2018-07-25T16:48:24Z

This doesn't seem to actually make a progress bar:

vmware-iso output will be in this color.

==> vmware-iso: Downloading or copying ISO
    vmware-iso: Downloading or copying: http://old-releases.ubuntu.com/releases/14.04.1/ubuntu-14.04.1-server-amd64.iso
    vmware-iso:  0 B / 572.00 MiB    0.00%
    vmware-iso:  3.33 MiB / 572.00 MiB    0.58%
    vmware-iso:  3.74 MiB / 572.00 MiB    0.65%
    vmware-iso:  4.34 MiB / 572.00 MiB    0.76%
    vmware-iso:  5.17 MiB / 572.00 MiB    0.90%
    vmware-iso:  5.92 MiB / 572.00 MiB    1.03%
    vmware-iso:  6.32 MiB / 572.00 MiB    1.10%

SwampDragons · 2018-07-25T16:49:13Z

If I cancel the download, then the progress bar appears:

vmware-iso:  0 B / ? [-----=]   0.00%
==> vmware-iso: Interrupt received. Cancelling download...

arizvisa · 2018-07-28T03:59:32Z

Yea, sorry. Progress-bar was probably not the correct term to submit this PR as. When I took the suggestion, I really only cared about the progress rate and set the width too small (25) so it doesn't render it due to the bit transfer rate occupying most of that width. Plus, I'm not sure how to determine the console width to set a reasonable size as the console between windows and posix are both vastly different (no terminal escape codes).

That function that I mentioned lets you customize how you guys would want it to look, though. I can fiddle with it, but it's purely aesthetic and I won't be able to get to it until I finish this other thing I'm working on. I suppose it's up to the community how they'd want the progress bar to look.

If you don't care, you can just empty out that GetDefaultProgressBar, and then it'll look like the following: https://github.com/cheggaaa/pb. But then you get no bit transfer rate which I think is more valuable information.

SwampDragons · 2018-08-02T18:36:30Z

Hmm. I guess I'd misunderstood the status and intention of this PR, then. I don't think we're gaining that much with this PR over the current download UI, and if all you care about is displaying the rate we can get that without adding a new dependency. I'm going to close this after all. Sorry for dragging this out so long.

arizvisa · 2018-08-04T12:40:00Z

Um. If you want a progress-bar it's literally setting a number from 25 to 80 (or 132). I'm pretty much just letting you know how to tweak the apperance, but if you're against the progress status. Tell me how you want me to make it look. Do you want '....complete!', or '====}', or '----->', etc.

The current progress UI only shows the estimated percentage of completion and not the transfer rate. This was one of the reasons why my previous patch to common/download.go added protocol transfers using goroutines. Plus, this dep is usable for all types of transfers.

…o the conversation on PR hashicorp#5851.

arizvisa · 2018-08-06T21:14:19Z

ok. I set the progress bar wider for you and put it into its own file so its more obvious that it can (and should) be tweaked. Also added it to the upload/download for the file provisioner. If you care, I can re-submit the PR.

Showing the amount of data transferred is significant sensor when troubleshooting network performance issues such as this winrmcp issue that you guys have been sitting on for a bit.

If you really don't care about having a progress-bar in packer, fine. But keep in mind that a progress-bar has been asked about for the past year now. So please take the time to finally add one. I don't make a living being a programmer or writing golang or anything like y'all do (I don't even use DevOps shit for that matter!) so it's not easy for me to take the time to write these things up and then fight to get them merged upstream like I've had to do with pretty much all the patches I've submitted.

arizvisa · 2018-08-15T01:09:42Z

it's a real shame that we can't go with our elegant solution as I've gotten kind of attached to the way it turned out after we refactored it.

but yea, i'm totally down with your suggestion. i haven't gotten a chance to look at your commits yet, but i'm totally down with calculating the name length and maybe leaving a comment to notify any devers that the progressbar length calculation is tied to any extra text that a packer.UI object might add to a message.

We can probably also calculate the length of PACKER_LOG as well so things still look good when logging is enabled.

SwampDragons · 2018-08-15T16:09:04Z

Yeah I'm sad about it too. Attaching it to the UI did seem more "beautiful" in theory; but ultimately changing the interface is probably not worth it because it'll break third party plugins for just a little extra debugging information and cosmetic change. 😢

…o the conversation on PR hashicorp#5851.

arizvisa · 2018-08-19T06:12:18Z

ok. rebased it. went through another refactor to move things from packer.Ui back into common. split up terminal-specific code in case someone wants to extend it later. added tests for everything, and also incorporated your hack. I don't have osx to test it, but it seems to work on my systems. lmk if there's anything else.

SwampDragons · 2018-08-20T18:14:24Z

post-processor/vagrant-cloud/client.go

@@ -102,33 +102,33 @@ func (v VagrantCloudClient) Delete(path string) (*http.Response, error) {
 }

 func (v VagrantCloudClient) Upload(path string, url string) (*http.Response, error) {
-	file, err := os.Open(path)


These changes don't seem related to the progress bar change.

yea. you're right. i'll find which commit added this and remove it.

you might want to fix this tho as it's a file handle leak.

SwampDragons · 2018-08-20T18:15:47Z

vendor/vendor.json

 		{
 			"checksumSHA1": "X2/71FBrn4pA3WcA620ySVO0uHU=",
 			"path": "github.com/creack/goselect",
 			"revision": "528c74964609a58f7c17471525659c9b71cd499b",
 			"revisionTime": "2018-02-10T03:43:46Z"
 		},
 		{
-			"checksumSHA1": "/5cvgU+J4l7EhMXTK76KaCAfOuU=",
+			"checksumSHA1": "Lf3uUXTkKK5DJ37BxQvxO1Fq+K8=",


is this change necessary? When I build locally it seems to be updating itself to a different SHA: "checksumSHA1": "dvabztWVQX8f6oMLRyv4dLH+TGY=",

ok, it's from a failed revert since this was originally from like forever ago. rm'ing it.

SwampDragons

Okay apart from the two very minor questions below I think we're good. Finally 😅Thanks for bearing with me through this headache. Do you think you can squash so we have a cleaner history without quite as many commits coming into this one merge?

…added some missing arguments that were missed during the rebase. Modified the default progress bar's width to 80 as a result of the conversation onf PR hashicorp#5851.

…added some missing arguments that were missed during the rebase. Modified the default progress bar's width to 80 as a result of the conversation on PR hashicorp#5851.

…th using the minimum length from a packer.UI and the terminal dimensions via kernel32.GetConsoleScreenBufferInfo or an ioctl (TIOCGWINSZ) to "/dev/tty".

…actored it so that the terminal width is calculated based on each interface which returns a custom progressbar specific to its ui.

…e packer.Ui implementations. Split up the terminal-related functions into a separate terminal.go and calculate the progress bar width by traversing through packer.Ui to avoid the issue with github.com/ugorji/go/codec serializing private members (or unsafe pointers) of structs. Shuffled some arguments around in getConsoleScreenBufferInfo in common/terminal_windows.go so that the interface forces the user to correctly declare a _CONSOLE_SCREEN_BUFFER_INFO type.

…with packer.rpc.Ui not exporting any information about what it's doing to anybody.

arizvisa · 2018-08-21T03:10:11Z

ok, it got pretty confusing during the rebase due to that revert of the rename from "CopyFile" to "inPlace" PR. but, I squashed out all the worthless commits, so it's down to just the 1 revert, the 3 major refactors and the vendor.json updates (7).

i'd like to retain some of this history (since this branch is tied to the fork in my gh acct), so if you want me to squash them all down to 1 to erase all of it, i'll probably need to re-submit the PR w/ a different branch.

ghost · 2020-03-31T02:04:56Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

arizvisa requested a review from a team as a code owner February 5, 2018 21:53

arizvisa force-pushed the multi-proto-progressbar branch from 57c6f91 to cfabf1c Compare February 5, 2018 21:56

arizvisa force-pushed the multi-proto-progressbar branch from cfabf1c to 9d4583d Compare February 14, 2018 17:53

arizvisa mentioned this pull request Feb 14, 2018

Improved the download progress bar. This supercedes the PercentProgress functionality. #5903

Closed

arizvisa mentioned this pull request Feb 14, 2018

Progress bar for vagrant-cloud upload #5838

Closed

arizvisa force-pushed the multi-proto-progressbar branch from 9d4583d to 2e7633b Compare April 27, 2018 06:29

arizvisa force-pushed the multi-proto-progressbar branch 2 times, most recently from b19b434 to ca543eb Compare July 24, 2018 19:47

SwampDragons added this to the upcoming-patch milestone Jul 24, 2018

SwampDragons added enhancement core Core components of Packer labels Jul 24, 2018

SwampDragons closed this Aug 2, 2018

arizvisa added a commit to arizvisa/hashicorp-packer that referenced this pull request Aug 6, 2018

Modified the default progress bar's width to 80. This is in regards t…

7d611ff

…o the conversation on PR hashicorp#5851.

SwampDragons reopened this Aug 6, 2018

arizvisa force-pushed the multi-proto-progressbar branch from dc0c2d1 to 2bea3ed Compare August 19, 2018 04:34

arizvisa added a commit to arizvisa/hashicorp-packer that referenced this pull request Aug 19, 2018

Modified the default progress bar's width to 80. This is in regards t…

e660e57

…o the conversation on PR hashicorp#5851.

arizvisa force-pushed the multi-proto-progressbar branch from edaced3 to a1f8eb5 Compare August 19, 2018 04:59

arizvisa added a commit to arizvisa/hashicorp-packer that referenced this pull request Aug 19, 2018

Modified the default progress bar's width to 80. This is in regards t…

a3b2109

…o the conversation on PR hashicorp#5851.

SwampDragons reviewed Aug 20, 2018

View reviewed changes

SwampDragons approved these changes Aug 20, 2018

View reviewed changes

arizvisa force-pushed the multi-proto-progressbar branch from 07d610d to 40b9fed Compare August 21, 2018 02:07

arizvisa force-pushed the multi-proto-progressbar branch from 40b9fed to 0faf58a Compare August 21, 2018 02:10

arizvisa force-pushed the multi-proto-progressbar branch from 0faf58a to 46b06eb Compare August 21, 2018 02:45

arizvisa added 7 commits August 20, 2018 21:55

Reverted removal of progress-bar that was done by commit 5d97b10 and …

16ecb3a

…added some missing arguments that were missed during the rebase. Modified the default progress bar's width to 80 as a result of the conversation on PR hashicorp#5851.

Updated vendor package github.com/cheggaaa/pb to v1.0.22

ac27e54

Added support for the progress bar to automatically determine its wid…

fd2fef8

…th using the minimum length from a packer.UI and the terminal dimensions via kernel32.GetConsoleScreenBufferInfo or an ioctl (TIOCGWINSZ) to "/dev/tty".

Moved the progress bar from common to the packer.UI interface and ref…

0f10032

…actored it so that the terminal width is calculated based on each interface which returns a custom progressbar specific to its ui.

Updated vendor package github.com/cheggaaa/pb to v1.0.25.

dc20883

Incorporated the hack demonstrated by @SwampDragons in order to deal …

cf9bbe3

…with packer.rpc.Ui not exporting any information about what it's doing to anybody.

arizvisa force-pushed the multi-proto-progressbar branch from 46b06eb to cf9bbe3 Compare August 21, 2018 02:55

SwampDragons approved these changes Aug 21, 2018

View reviewed changes

SwampDragons merged commit c1b50d3 into hashicorp:master Aug 21, 2018

azr mentioned this pull request Aug 24, 2018

download progress bar is not refreshing but sending newlines #6625

Closed

SwampDragons mentioned this pull request Sep 5, 2018

Add a Progress indicator for upload provisioners #6671

Closed

ghost locked and limited conversation to collaborators Mar 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add progress-bar to common/step_download.go #5851

Add progress-bar to common/step_download.go #5851

arizvisa commented Feb 5, 2018 •

edited by SwampDragons

Loading

arizvisa commented Feb 14, 2018

arizvisa commented Feb 14, 2018

arizvisa commented Jun 14, 2018

SwampDragons commented Jun 15, 2018

arizvisa commented Jun 15, 2018

SwampDragons commented Jul 23, 2018

arizvisa commented Jul 24, 2018

arizvisa commented Jul 24, 2018

arizvisa commented Jul 24, 2018

SwampDragons commented Jul 24, 2018

SwampDragons commented Jul 25, 2018

SwampDragons commented Jul 25, 2018

arizvisa commented Jul 28, 2018

SwampDragons commented Aug 2, 2018

arizvisa commented Aug 4, 2018

arizvisa commented Aug 6, 2018

arizvisa commented Aug 15, 2018

SwampDragons commented Aug 15, 2018

arizvisa commented Aug 19, 2018

SwampDragons Aug 20, 2018

arizvisa Aug 21, 2018

SwampDragons Aug 20, 2018

arizvisa Aug 21, 2018

SwampDragons left a comment

arizvisa commented Aug 21, 2018

ghost commented Mar 31, 2020

Add progress-bar to common/step_download.go #5851

Add progress-bar to common/step_download.go #5851

Conversation

arizvisa commented Feb 5, 2018 • edited by SwampDragons Loading

arizvisa commented Feb 14, 2018

arizvisa commented Feb 14, 2018

arizvisa commented Jun 14, 2018

SwampDragons commented Jun 15, 2018

arizvisa commented Jun 15, 2018

SwampDragons commented Jul 23, 2018

arizvisa commented Jul 24, 2018

arizvisa commented Jul 24, 2018

arizvisa commented Jul 24, 2018

SwampDragons commented Jul 24, 2018

SwampDragons commented Jul 25, 2018

SwampDragons commented Jul 25, 2018

arizvisa commented Jul 28, 2018

SwampDragons commented Aug 2, 2018

arizvisa commented Aug 4, 2018

arizvisa commented Aug 6, 2018

arizvisa commented Aug 15, 2018

SwampDragons commented Aug 15, 2018

arizvisa commented Aug 19, 2018

SwampDragons Aug 20, 2018

Choose a reason for hiding this comment

arizvisa Aug 21, 2018

Choose a reason for hiding this comment

SwampDragons Aug 20, 2018

Choose a reason for hiding this comment

arizvisa Aug 21, 2018

Choose a reason for hiding this comment

SwampDragons left a comment

Choose a reason for hiding this comment

arizvisa commented Aug 21, 2018

ghost commented Mar 31, 2020

arizvisa commented Feb 5, 2018 •

edited by SwampDragons

Loading