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

busyhandler execution sorting #772

Merged
merged 1 commit into from
May 29, 2020

Conversation

fopina
Copy link
Contributor

@fopina fopina commented May 25, 2020

Currently, busyHandler does no sorting, and the web UI doesn't either.
The result is a page where it is hard to follow any output as executions keep switching order:
Peek 2020-05-25 00-11

Sorting can be done both in the UI or in the busyHandler method.
In the UI it probably saves that (small yet existing) processing from dkron server, but then it shouldn't return an array.
As an API consumer I expect a JSON array response to have some sort of order (usually, from a DB, it would be the cheapest order, the insertion order).
Regardless of the sorting criteria, the same request with the same set of results should always have the same order.
So I'd say it could make sense to leave the UI to order the response but turn it into a mapping/hash (execution_name: execution) or simply sort in busyHandler with some common criteria such as jobName or startTime.

I chose the latter in this PR, let me know if you'd rather have another approach.

End result:

Peek 2020-05-25 01-49

@fopina
Copy link
Contributor Author

fopina commented May 26, 2020

probably member list (in the dashboard page) could use a sort as well ?

Copy link
Member

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@vcastellm
Copy link
Member

probably member list (in the dashboard page) could use a sort as well ?

It already does but in the UI https://github.com/distribworks/dkron/blob/master/templates/index.html.tmpl#L25

@vcastellm vcastellm merged commit bc22773 into distribworks:master May 29, 2020
@fopina fopina deleted the fopina-busy-order branch June 8, 2020 23:41
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.

2 participants