Skip to content

Support high build time variation in progress bar #4941

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

Merged
merged 12 commits into from
Nov 17, 2022
Merged

Conversation

ammario
Copy link
Member

@ammario ammario commented Nov 7, 2022

Resolves #4676

This PR changes the Progress Bar's behavior so that it shows an estimate range (and no progress slider) when estimate variation is too high.

image

@ammario ammario mentioned this pull request Nov 7, 2022
@ammario ammario requested a review from kylecarbs November 8, 2022 00:43
Copy link
Member

@kylecarbs kylecarbs left a comment

Choose a reason for hiding this comment

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

Nothing is required, but I think we could benefit from moving more to the BE.

@@ -26,6 +26,7 @@ import (
)

func TestLoadTest(t *testing.T) {
t.Skipf("This test is flakey. See https://github.com/coder/coder/issues/4942")
Copy link
Member

Choose a reason for hiding this comment

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

I already disabled the flakey test in another PR.

Comment on lines 52 to 54
// If variation is too high (and greater than second), don't show
// progress bar and give range.
const highVariation = stddev / median > 0.1 && highGuess - lowGuess > 1
Copy link
Member

Choose a reason for hiding this comment

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

I might do this logic on the backend instead and return simpler API values. In the current format, it's not possible for our CLI to display a progress bar or estimated time remaining without duplicating this logic.

@github-actions
Copy link

This Pull Request is becoming stale. In order to minimize WIP, prevent merge conflicts and keep the tracker readable, I'm going close to this PR in 3 days if there isn't more activity.

@github-actions github-actions bot added the stale This issue is like stale bread. label Nov 16, 2022
@github-actions github-actions bot removed the stale This issue is like stale bread. label Nov 17, 2022
@ammario ammario marked this pull request as ready for review November 17, 2022 16:44
@ammario ammario requested a review from a team as a code owner November 17, 2022 16:44
@ammario ammario requested review from Kira-Pilot and removed request for a team November 17, 2022 16:44
@ammario ammario changed the title Don't show progress bar when data inconclusive Support high build time variation in progress bar Nov 17, 2022
@ammario ammario requested a review from kylecarbs November 17, 2022 16:51
@ammario ammario enabled auto-merge (squash) November 17, 2022 16:51
@ammario
Copy link
Member Author

ammario commented Nov 17, 2022

@kylecarbs — agree it would've been better to add this into the backend, but I want to merge into the frontend for now since it's not the most important work I could be doing.

@ammario ammario merged commit acf34d4 into main Nov 17, 2022
@ammario ammario deleted the pb-improvements branch November 17, 2022 16:56
@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 2022
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.

progress bar: build running is inaccurate
2 participants