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

Improve logs (introduce a progress-bar for tty devices) #84

Closed
brandonkal opened this issue Jun 24, 2020 · 9 comments
Closed

Improve logs (introduce a progress-bar for tty devices) #84

brandonkal opened this issue Jun 24, 2020 · 9 comments
Assignees
Labels
hacktoberfest type:enhancement Small feature requests / adjustments

Comments

@brandonkal
Copy link

Running the Go example, earth floods the terminal with lines like this:

+base | sha256:61b11419476186415e86da00c53c938fd32dc5904a889e1c3d40e238d69d168e 11%
+base | sha256:61b11419476186415e86da00c53c938fd32dc5904a889e1c3d40e238d69d168e 11%
+base | sha256:61b11419476186415e86da00c53c938fd32dc5904a889e1c3d40e238d69d168e 12%
+base | sha256:61b11419476186415e86da00c53c938fd32dc5904a889e1c3d40e238d69d168e 12%
+base | sha256:61b11419476186415e86da00c53c938fd32dc5904a889e1c3d40e238d69d168e 12%
+base | sha256:61b11419476186415e86da00c53c938fd32dc5904a889e1c3d40e238d69d168e 12%
+base | sha256:61b11419476186415e86da00c53c938fd32dc5904a889e1c3d40e238d69d168e 12%
+base | sha256:61b11419476186415e86da00c53c938fd32dc5904a889e1c3d40e238d69d168e 12%

Though it is a lot more of that than I've included here. I would expect this as a progress bar replacing the line on update rather than drawing a new line.

go build is silent on success so this is a lot messier.

@vladaionescu
Copy link
Member

Makes sense, Brandon. We need to improve this.

(I believe those specific lines are from the progress of pulling some docker image - this can be made less verbose.)

@vladaionescu vladaionescu added the type:enhancement Small feature requests / adjustments label Jun 24, 2020
@brandonkal
Copy link
Author

Yes that is from pulling the image. My internet connection is slow so I get a higher amount of those lines.
With a tty, a simple wget-like progress bar would be better.
Without a tty, one solution is to filter out duplicate lines (that at least reduces it down to 100 lines for each pull).

@abennett abennett mentioned this issue Aug 10, 2020
@alexcb
Copy link
Collaborator

alexcb commented Aug 10, 2020

#143 changes the behaviour to only print out the progress every 5 seconds.

I agree that a tty progress bar would be nice to implement one day, so maybe we should leave this issue open?

@alexcb alexcb changed the title Improve logs Improve logs (introduce a progress-bar for tty devices) Aug 10, 2020
@Gituser143
Copy link

Hey @vladaionescu! Can I work on this issue?

@vladaionescu
Copy link
Member

Sounds good @Gituser143. This is where progress is printed BTW: https://github.com/earthly/earthly/blob/master/builder/solver_monitor.go#L178. Keep in mind that there may be multiple progress bars ongoing at a time and that the printing itself is being worked on in parallel: #338.

CC @MadhavJivrajani

@Gituser143
Copy link

Thank you @vladaionescu! I'll get to it...

@Gituser143
Copy link

Gituser143 commented Oct 2, 2020

@vladaionescu I've implemented a simple progress bar. It's declared here and values are updated like this. Let me know if I can open a PR so that you can see the changes and tell me what changes I'll have to make.

My overall implementation looks kind of like this:

var on sync.Once
progressBar := pb.New(100)
for _, vs := range ss.Statuses {
  .
  .
  .  
  progress := int(0)
  if vs.Total != 0 {
    progress = int(100.0 * float32(vs.Current) / float32(vs.Total))
  }
  if vs.Completed != nil {
    progress = 100
  }
  if vm.shouldPrintProgress(progress) {
    .
    .
    .
    on.Do(func() {
      vm.console.Printf("%s\n", vs.ID)
      progressBar.Start()
    })
    progressBar.SetCurrent(int64(progress))
  }
}
progressBar.Finish()

@vladaionescu
Copy link
Member

It's a good start, however you probably need a separate progress bar for each of the items in ss.Statuses. These are multiple activities taking place in parallel and the relevant progress bars need to be updated in parallel. Feel free to open a PR even if it's WIP.

@vladaionescu
Copy link
Member

This has been implemented and will be in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest type:enhancement Small feature requests / adjustments
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants