Empty progress bars never finish about mpb HOT 23 CLOSED

Well that's intended behaviour. It happens when total <= 0 in b := p.AddBar(total). mpb treats such case as total is unknown, and a user suppose to call b.Completed() method after work is done, see this gist for example.
Why not just return if total <= 0?
Suppose one want to download a file over http. Some http servers may report size 0, but resp.Body is not empty. Usually this happens when http server doesn't support resume. So rather than return silently, mpb fallbacks to simple spinner bar, and expects user to call b.Completed() method, after job is done.

from mpb.

I've implemented a fix at issue7fix branch, which will call b.Completed() implicitly.
Can you please checkout this branch, and check if it works for you?

from mpb.

I can confirm that this fixes the exit freeze, however it does display things oddly

foobar                                      0b /  0b [/] -9223372036854775808 %

Thanks for the quick fix though!

from mpb.

Can you show your code? How do you initialise a bar?

from mpb.

Certainly, the code is here:

https://github.com/kormoc/bitrot_scanner/blob/master/checksumHelpers.go#L53

from mpb.

Yes negative percentage is odd looking. I've fixed that, and merged to master. You can checkout.

from mpb.

Looking at your code

PrependCounters("%3s / %3s", mpb.UnitBytes, 18, mpb.DwidthSync|mpb.DextraSpace)

I assume you use multiple bars. For single bar, option mpb.DwidthSync has no effect.

from mpb.

As for simple spinner, the [/] part, it cycles through a set of -\|/ characters. Maybe I should let user to decide whether he/she wants this spinner for such edge case (total <= 0)? It is on by default, what do you think?

from mpb.

Yes, we (optionally) use multiple bars. I don't mind the spinner in this case. Thanks much for the quick fix!

from mpb.

You're welcome, thanks for reporting!

from mpb.

Hi, sorry to dig this issue out of its grave but it looks like this "problem" is still present with current versions (tested with v4.9.1). The code has much changed since this issue's fix commit, I tried to track down the code but can't seem to find it in the current code base so I wonder if it may have been forgotten during a refactoring maybe?

Our actual implementation is here: https://github.com/exoscale/cli/blob/master/cmd/sos_upload.go#L170-L194
If we pass the raw file reader to PutObjectWithContext() method it works fine with empty files, but gets stuck if we pass the bar.ProxyReader(f) reader.

from mpb.

Hi, you're right, Read method of proxyReader never checked EOF. Should be fixed with this commit. Can you pls check, and confirm if it's fixed?
I'll bump tag after your confirmation.

from mpb.

@vbauerster tested with this commit: no improvement.

from mpb.

I've simulated empty file/reader in io example, by changing this line to total := 0 and there is no freeze.

from mpb.

Are you sure your build has picked up aforementioned commit?

from mpb.

Yes:

$ git diff go.mod
diff --git a/go.mod b/go.mod
index efb439f..5a23218 100644
--- a/go.mod
+++ b/go.mod
@@ -23,7 +23,7 @@ require (
        github.com/spf13/cobra v0.0.3
        github.com/spf13/pflag v1.0.3
        github.com/spf13/viper v1.3.1
-       github.com/vbauerster/mpb/v4 v4.8.4
-       golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5
+       github.com/vbauerster/mpb/v4 v4.9.2-0.20190806152146-08e821e86ad3
+       golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4
        gopkg.in/alecthomas/kingpin.v3-unstable v3.0.0-20180810215634-df19058c872c // indirect
 )

$ git diff vendor/github.com/vbauerster/mpb/v4/proxyreader.go
diff --git a/vendor/github.com/vbauerster/mpb/v4/proxyreader.go b/vendor/github.com/vbauerster/mpb/v4/proxyreader.go
index d2692cc..1e172be 100644
--- a/vendor/github.com/vbauerster/mpb/v4/proxyreader.go
+++ b/vendor/github.com/vbauerster/mpb/v4/proxyreader.go
@@ -18,5 +18,9 @@ func (pr *proxyReader) Read(p []byte) (n int, err error) {
                pr.bar.IncrBy(n, time.Since(pr.iT))
                pr.iT = time.Now()
        }
+       if err == io.EOF {
+               current := pr.bar.Current()
+               pr.bar.SetTotal(current, true)
+       }
        return
 }

$ make
go clean
rm -rf exo contrib/completion manpage
go build -ldflags "-s -w -X main.version=dev -X main.commit=b7f5c7dd64d3c8c08d6994519cb2d76ffabebc39" -mod=vendor -o exo

$ ./exo version
exo dev b7f5c7dd64d3c8c08d6994519cb2d76ffabebc39 (egoscale 0.18.1)

$ touch blah
$ ./exo sos upload marc-test blah
0s 0 % [--------------------------------------------------------------] blah
# ^^^ gets stuck ^^^

from mpb.

You can perform the same test on your end by checking out our repository: https://github.com/exoscale/cli

from mpb.

It asks for config which I don't have. I wonder if Read method is called at all on empty file. You can troubleshoot by putting fmt.Fprintln(os.Stderr, "read called") here, then run like ./exo 2> debug.txt. I recommend using gohack for that.

from mpb.

Tested with your instructions: the debug.txt is empty.

from mpb.

Tested with your instructions: the debug.txt is empty.

Wait nevermind, I think the debug line has not been taken by the compiler via gohack. Let me try again.

from mpb.

This time I'm positive the modified file is actually used because I got compilation error complaining about missing fmt/os package imports.

Sadly, the debug.txt remains empty:

$ git diff vendor/github.com/vbauerster/mpb/v4/proxyreader.go
diff --git a/vendor/github.com/vbauerster/mpb/v4/proxyreader.go b/vendor/github.com/vbauerster/mpb/v4/proxyreader.go
index d2692cc..d4b142e 100644
--- a/vendor/github.com/vbauerster/mpb/v4/proxyreader.go
+++ b/vendor/github.com/vbauerster/mpb/v4/proxyreader.go
@@ -1,7 +1,9 @@
 package mpb

 import (
+       "fmt"
        "io"
+       "os"
        "time"
 )

@@ -13,10 +15,15 @@ type proxyReader struct {
 }

 func (pr *proxyReader) Read(p []byte) (n int, err error) {
+       fmt.Fprintln(os.Stderr, "read called")
        n, err = pr.ReadCloser.Read(p)
        if n > 0 {
                pr.bar.IncrBy(n, time.Since(pr.iT))
                pr.iT = time.Now()
        }
+       if err == io.EOF {
+               current := pr.bar.Current()
+               pr.bar.SetTotal(current, true)
+       }
        return
 }

$ make
go clean
rm -rf exo contrib/completion manpage
go build -ldflags "-s -w -X main.version=dev -X main.commit=b7f5c7dd64d3c8c08d6994519cb2d76ffabebc39" -mod=vendor -o exo


$ ./exo sos upload marc-test blah 2>debug.txt
0s 0 % [--------------------------------------------------------------] blah
^C 
$ cat debug.txt
$

from mpb.

Well it means that minioClient doesn't handle it properly, however as workaround you may consider checking empty file manually and call bar.SetTotal(0, true) afterwards. Long term fix is to report upstream to minio lib.

from mpb.

Well, indeed your workaround works – thank you for that. I'll go ahead and open an issue at Minio, you can close this one.

from mpb.