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

feat: Use progress bar to display docker output #913

Merged
merged 18 commits into from
May 28, 2024
Merged

feat: Use progress bar to display docker output #913

merged 18 commits into from
May 28, 2024

Conversation

tianfeng92
Copy link
Contributor

@tianfeng92 tianfeng92 commented May 24, 2024

Description

Use progress spinner to display docker output.
The output from the Docker API consists of groups of steps, each group having an identical ID and logging in parallel.
Use a spinner to display steps without detailed progress and a progress bar to show the progress of the pushing status.

Output:

$ saucectl-local docker push us-west4-docker.pkg.dev/sauce-hto-p-jy6b/sauce-devx-team-sauce/devx-test:3.0
Pushing 7e0d10e4156a 100% [=============================================================================>] (62 kB/s)
Pushing 9c7bbe04a91a 100% [=============================================================================>] (24 MB/s)
Pushing 666ebea2205a 100% [==============================================================================>] (54 B/s)
Pushing e53a50d4c628 100% [=============================================================================>] (63 kB/s)
Pushing 9847ff9c0412 100% [=============================================================================>] (375 B/s)
Successfully pushed the Docker image!

@tianfeng92 tianfeng92 added the enhancement New feature or request label May 24, 2024
@tianfeng92 tianfeng92 marked this pull request as ready for review May 24, 2024 23:38
@tianfeng92 tianfeng92 requested a review from a team as a code owner May 24, 2024 23:38
internal/cmd/docker/push.go Outdated Show resolved Hide resolved
@tianfeng92 tianfeng92 requested a review from mhan83 May 27, 2024 20:41
@mhan83
Copy link
Contributor

mhan83 commented May 27, 2024

Got this when trying it out:

🍖 ./saucectl docker push us-west4-docker.pkg.dev/sauce-hto-p-jy6b/sauce-devx-team-sauce/testcafe-browser-provider:latest1
Pushing   0% [                                                                                                                                                                                                                     >]  [0s:0s]Error: current number exceeds max

@tianfeng92
Copy link
Contributor Author

Got this when trying it out:

🍖 ./saucectl docker push us-west4-docker.pkg.dev/sauce-hto-p-jy6b/sauce-devx-team-sauce/testcafe-browser-provider:latest1
Pushing   0% [                                                                                                                                                                                                                     >]  [0s:0s]Error: current number exceeds max

hmm.. 🔍

Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com>
Comment on lines +143 to +144
// Note: The Docker API may return a current value greater than total. To prevent breaking the progress bar,
// only update when the current is less than total.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does that really break the progress bar?

This works fine for me, even when the loop goes beyond the max total of the bar.

func main() {
	bar := progressbar.Default(50)
	for i := 0; i < 100; i++ {
		_ = bar.Add(1)
		time.Sleep(40 * time.Millisecond)
	}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not using Add. Set64 can break it.

func main() {
	bar := progressbar.Default(50)
	bar.Set64(3000)
	err := bar.Finish()
	fmt.Println("err: ", err)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/schollz/progressbar/blob/v3.14.3/progressbar.go#L551C1-L557C3

The added number is not checked. So current number is greater than total after this piece of code.

Add can also break it.

func main() {
	bar := progressbar.Default(50)

	bar.Add(51)
	err := bar.Finish()
	fmt.Println("err: ", err)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😄 100% break

func main() {
	bar := progressbar.Default(50)
	for i := 0; i < 100; i++ {
		err := bar.Add(3)
		if err != nil {
			fmt.Println("err: ", err)
			break
		}
		time.Sleep(40 * time.Millisecond)
	}
}

@tianfeng92 tianfeng92 merged commit 77a9777 into main May 28, 2024
16 checks passed
@tianfeng92 tianfeng92 deleted the DEVX-2808 branch May 28, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants