-
Notifications
You must be signed in to change notification settings - Fork 645
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
[5.x] Display inactive supervisors in dashboard #1286
[5.x] Display inactive supervisors in dashboard #1286
Conversation
8df7018
to
acf3aa2
Compare
acf3aa2
to
e876265
Compare
Thanks. Can you add some screenshots of this? |
Hi @PrinsFrank and @driesvints, Great contribution! However, there are scenarios where it might be better to hide them. For example; I have projects (and others I know have them too) where they intentionally do not add a supervisor to certain workers (e.g. via the Horizon env variable). Maybe a setting toggle is useful, or we should add a config setting to |
@bjarn Do you have an example config and a screenshot of one such scenario? We have a similar setup running and the supervisors that should not run for a specific provisioning plan are not shown. |
@PrinsFrank You were fast! Just posted one at the same time when you replied. Some parts of the config where I define the environments and where it gets it from. /*
| ------------------------------------------------------------------
| Horizon Environment
| ------------------------------------------------------------------
|
| This value determines Horizon's environment
|
*/
'env' => env('HORIZON_ENV'),
'environments' => [
'ams' => [
'ams-worker-1' => [
'connection' => 'redis',
'queue' => ['...-ams'],
....
],
],
'mia' => [
'mia-worker-1' => [
'connection' => 'redis',
'queue' => ['...-mia'],
....
],
],
'syd' => [
'syd-worker-1' => [
'connection' => 'redis',
'queue' => ['...-syd'],
....
],
],
'fns' => [
'fns-app-1' => [
'connection' => 'redis',
'queue' => ['...'],
....
],
], I do think I see a pattern. It's not showing 'mia' and 'syd' as missing in the other workers, just the 'fns' ones, which is the one hosting the Horizon dashboard. |
@bjarn Thanks, I am able to reproduce this, thanks! Let's see if I can fix this quickly. |
To clarify: The intention was to show supervisors for environments where the supervisors were intended to run on that environment but where the supervisor had crashed. But running multiple machines with different environments was not working correctly (with the --enviroment flag or using a different environment in the .env). When this PR gets merged that will now also work, and missing supervisors will only be displayed for supervisors that are actually missing. If you want you can add my fork as a vcs repository and require the branch to verify the fix: composer config repositories.foo vcs "https://github.com/prinsfrank/laravel-horizon"
composer require laravel/horizon:dev-fix-inactive-supervisors-shown-on-different-env |
That's great! I'll try to test it tomorrow :)! And makes sense! Thanks a lot for fixing this so quickly. |
When running horizon spread over a lot of machines and with a lot of supervisors per machine it becomes difficult to see what machine misses what specific supervisor. (You have to count the number of rows and then find the one that is missing).
This PR adds any missing supervisor in the overview for the machine, indicated with a cross icon so It's visible that the supervisor is not running. (The number of processes is also 0)
I think I have a fix for the underlying missing supervisors issue, we're running that right now in production. If so, a PR will follow somewhere next week.