8000 Show task progress in visualizer workers tab. by riga · Pull Request #2932 · spotify/luigi · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Show task progress in visualizer workers tab. #2932

New issue
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
0