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

Show task progress in visualizer workers tab. #2932

Merged
merged 2 commits into from
May 19, 2020

Conversation

riga
Copy link
Contributor

@riga riga commented Apr 18, 2020

Description

This PR adds the task progress percentage as a bootstrap progress bar in a new column to the table of running tasks in the "Workers" tab:

progress

The value is even updated when the message modal of the specific task is open (which already had the ability to update task messages and progress via polling).

I didn't include the new column in the actual "Task List" tab, because it is probably only important for currently running tasks.

Motivation and Context

When dealing with a couple long-running tasks, it is a nice feature to see the progress per task already in the worker tab.

Have you tested this? If so, how?

The behavior is rather static, and I couldn't find existing tests that check similar features. However, I verified that the automatic updating via the refreshInterval of the opened modal is working properly and handles also null values.

@dlstadther
Copy link
Collaborator

I've never dealt with the UI portion of Luigi. Do you have a colleague you can tag for review? Or perhaps a previous Luigi UI contributor?

I like the idea here.

@riga
Copy link
Contributor Author

riga commented Apr 23, 2020

I actually contributed a few UI related PRs such as task messages, worker processes customization, scheduler → worker communication, etc, but I think avoiding self-review is the point here ;)

@yrath @bfis Could you maybe give it a try? I prepared some test tasks to run in this gist.

@yrath
Copy link

yrath commented May 5, 2020

Sorry for the late reply. I've checked out the PR based on the gist, everything worked as expected and looks good to me.

@riga
Copy link
Contributor Author

riga commented May 12, 2020

@yrath Thanks for testing!

@riga
Copy link
Contributor Author

riga commented May 19, 2020

@dlstadther Is this ok for you or should I provide additional tests?

@dlstadther
Copy link
Collaborator

Fellow review and testing works for me here. I'll merge.

@dlstadther dlstadther merged commit 8ef2573 into spotify:master May 19, 2020
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.

3 participants