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

Projects
None yet
4 participants
@michaeldyrynda
Copy link
Contributor

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

This comment has been minimized.

Copy link
Member

driesvints commented Jan 9, 2019

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 michaeldyrynda force-pushed the michaeldyrynda:2.0 branch from 6d6cd52 to 7621e1c Jan 9, 2019

@michaeldyrynda

This comment has been minimized.

Copy link
Contributor Author

michaeldyrynda commented Jan 10, 2019

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.

@michaeldyrynda michaeldyrynda force-pushed the michaeldyrynda:2.0 branch from e8dfb57 to cff15db Jan 10, 2019

@JayBizzle

This comment has been minimized.

Copy link
Contributor

JayBizzle commented Jan 10, 2019

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

This comment has been minimized.

Copy link
Contributor Author

michaeldyrynda commented Jan 10, 2019

I'm happy with this now 👍

@driesvints

This comment has been minimized.

Copy link
Member

driesvints commented Jan 11, 2019

Thanks for your work on this man 👍

@taylorotwell taylorotwell merged commit 7144bd6 into laravel:2.0 Jan 12, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment