framework
framework copied to clipboard
feat: expose queue driver, schedule status
Changes proposed in this pull request: An effort to promote the use of the scheduler, this proposes exposing it's basic status to the dashboard. Possible statuses are
Never runActiveInactive
A simple timestamp is stored in settings, with a check to see if the last run was > 5 minutes ago.
Additionally, we expose the driver in use for the Queue, using shared logic from InfoCommand
Reviewers should focus on: As other info added to the admin payload, strings are hard coded. Should these perhaps become translations? I'd lean towards that, actually.
Screenshot
Active:

Inactive:

Never run:

Redis queue driver:

edit - added info link to scheduler setup information

Necessity
- [ ] Has the problem that is being solved here been clearly explained?
- [ ] If applicable, have various options for solving this problem been considered?
- [ ] For core PRs, does this need to be in core, or could it be in an extension?
- [ ] Are we willing to maintain this for years / potentially forever?
Confirmed
- [ ] Frontend changes: tested on a local Flarum installation.
- [ ] Backend changes: tests are green (run
composer test). - [ ] Core developer confirmed locally this works as intended.
- [ ] Tests have been added, or are not appropriate here.
Required changes:
- [ ] Related documentation PR: (Remove if irrelevant)
- [ ] Related core extension PRs: (Remove if irrelevant)
Doesn't laravel dispatch events when the scheduled tasks run?
With scheduled tasks requiring registration we can only show scheduled tasks health when there actually tasks scheduled.
I don't know how much this conflicts with our plans for an advanced tab for queues, and other things mostly for larger communities ...
Doesn't laravel dispatch events when the scheduled tasks run?
It does, ScheduledTaskStarting, ScheduledTaskFinished and ScheduledTaskFailed, which indeed could be used for a more detailed insight into scheduled tasks for sure. Perhaps that's something we'd like to introduce as well? The aim for this high level status indicator is more about "Is the schedule:run command being trigged regularly"
With scheduled tasks requiring registration we can only show scheduled tasks health when there actually tasks scheduled.
This implementation is not dependent on any tasks being registered, (see above comment).
I don't know how much this conflicts with our plans for an advanced tab for queues, and other things mostly for larger communities ...
I don't see any information about these plans on the roadmap or issues, perhaps I missed this? Could you perhaps point me in the direction of this so I can take a look?
Would it be worth adding warning icons for inactive or never run, warning of the potential consequences?
Yeah, I think that's a good call 👍
The advanced admin page had conversations here and there but nothing concrete written yet, although Daniël did push some code for it at some point though. So we don't yet know for sure what it will be like and I don't know when it will be tackled.
So I think it's fine to move with this, for now, I like the additional info. I haven't yet looked at the code, but I think we should really move the two discussions about setting up Scheduler(linked to in this pr) and Queues(linked to by the package manager) to the docs and link to the docs instead, though not necessarily blocking this PR.
Agreed. I do still believe it only makes sense to show the status for scheduler in case there are at least commands registered to run with it aka there's a need for it to run. Otherwise it will just confuse people about a need for the scheduler which doesn't even exist. Scenario's like "I have an error with X, but I noticed the scheduler isn't running so how can I run it?" will probably happen where X is unrelated to any scheduled task.
In order to maintain a consistent look, how about introducing an additional status for the scheduler element - Not required, which will be displayed when there are no tasks registered?
This would still keep the need for a schedule present in the forum admin's mind, but would make it clear that with the current set of extensions installed it is not needed, therefore would not cause unneccessary support/help requests?
I agree with Daniël, I prefer only showing the status if it's necessary rather then adding more statuses as that sounds like over complicating it for the end user.
I am starting to like this a lot. I also think it suffices for a first version, but I also think we need to make things more idiot proof and more intuitive in the future (2.x or sooner). The issue I feel is that we try to cramp too much in that little status bar, an advanced page would be far more helpful if we add more information on what for instance a scheduler is, why it is needed and why this seems to be required now. We want to prevent too much support load, and for that reason we need to be transparent about these things.
I am starting to like this a lot. I also think it suffices for a first version, but I also think we need to make things more idiot proof and more intuitive in the future (2.x or sooner). The issue I feel is that we try to cramp too much in that little status bar, an advanced page would be far more helpful if we add more information on what for instance a scheduler is, why it is needed and why this seems to be required now. We want to prevent too much support load, and for that reason we need to be transparent about these things.
Absolutely agree with you.