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

Timer status display #537

Merged
merged 4 commits into from Dec 8, 2016
Merged

Conversation

BrianHicks
Copy link
Contributor

This adds an incremental status display for long-running tasks.

Fixes #269

@BrianHicks BrianHicks force-pushed the feature/incremental-status-display branch from f083d20 to 2aa1f44 Compare December 6, 2016 19:16
slog.Info("got status")
}

if resp.Run == pb.StatusResponse_FINISHED {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could probably be moved to the above if else block since it is regarding resp.Run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to switch statements in both cases. Gotta take my own advice :D

@@ -109,6 +114,7 @@ can be done separately to see what needs to be changed before execution.`,
"id": resp.Meta.Id,
})
if resp.Run == pb.StatusResponse_STARTED {
timer.AddTimer(resp.Meta.Id + ": " + resp.Stage.String())
slog.Info("got status")
} else {
slog.Debug("got status")
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you intend to change Debug to Info here also?

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 actually forgot to change it back to Debug... I was just debugging.

func (td *TimerDisplay) Message() string {
td.once.Do(td.init)

out := "\nActive:\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed this output is sometimes displayed on one line and sometimes displayed twice, with the second a few lines below. The second instance flashes on and off the screen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's a race condition in the library we're using. I'm not sure what's causing it, but it seems to be an issue with multi-line displays. Unfortunately, it looks awful on one line so I think we're just going to have to deal with it :\

@arichardet arichardet merged commit d0fef40 into master Dec 8, 2016
@arichardet arichardet deleted the feature/incremental-status-display branch December 8, 2016 19:26
BrianHicks pushed a commit that referenced this pull request Dec 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants