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

[2.0] Handle configured 'trim' values in dashboard #461

Merged
merged 7 commits into from
Jan 12, 2019

Conversation

michaeldyrynda
Copy link
Contributor

@michaeldyrynda michaeldyrynda commented Jan 9, 2019

I'm opening this PR now to get some feedback on implementation.

Currently, Horizon allows you to configure how long recent and failed jobs are kept for statistical purposes before being trimmed (horizon.trim.recent and horizon.trim.failed).

The trouble with setting these currently is that the dashboard will always say Jobs past hour and Failed jobs past hour, irrespective of the configured values.

This is a first pass of making that label dynamic on the dashboard, by parsing the number of days or hours and displaying the message accordingly.

Current

Current

Proposed

Proposed

Note, I haven't compiled the CSS/JS assets.

@driesvints driesvints changed the title WIP: Handle configured 'trim' values in dashboard [2.0] WIP: Handle configured 'trim' values in dashboard Jan 9, 2019
@driesvints
Copy link
Member

This looks good but perhaps something more for 3.0 because the new methods on the JobRepository could be a breaking change for people implementing their own repository. Not sure how many people actually do this though...

@michaeldyrynda
Copy link
Contributor Author

Per @driesvints' comments above, I've rewritten this to be done in the Dashboard.vue component entirely, rather than making changes to the interfaces / repository classes.

@JayBizzle
Copy link
Contributor

This is much needed, had a bash at this myself a couple of months ago (see #393)

Your implementation is slightly better though 👍

@michaeldyrynda michaeldyrynda changed the title [2.0] WIP: Handle configured 'trim' values in dashboard [2.0] Handle configured 'trim' values in dashboard Jan 10, 2019
@michaeldyrynda
Copy link
Contributor Author

I'm happy with this now 👍

@driesvints
Copy link
Member

Thanks for your work on this man 👍

@taylorotwell taylorotwell merged commit 7144bd6 into laravel:2.0 Jan 12, 2019
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.

4 participants