Fix broken /admin/background-tasks web UI when using Sentinel #670
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tested in context of NetBox 4.1.3 and am able to confirm that this fixes issue #642 and adds back Sentinel support. Thank you @NeodymiumFerBore 👍 |
|
@NeodymiumFerBore PR #671 fixed it for me as well 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An attempt to fix #642.
Bug recap
When using Sentinel, in
get_scheduler_statisticsfunction,queue.connection.connection_pool.connection_kwargsdoes not have ahostkey, making the function to raise aKeyErrorexception when it tries to defineconn_keybased onconnection['host']. This makes the/admin/background-taskspage to return a 500 error, making it unusable.What does this fix do
This PR fixes this by building
conn_keyfrom all sentinels host:port pairs.With this fix, a scheduler configured with Sentinel appears like this (with
mymasterbeing the Sentinel Service, and0the configured db index for this scheduler):If you don't agree with this syntax, please let me know and I change it.
Alternative solution
An alternative solution would be to build the
conn_keyfrom the current Sentinel master for the given service, but I'm not sure what you prefer. It would make the code a bit easier to read, and would make the resulting string in the UI much shorter. But it's less clear about the config used for this scheduler.NeodymiumFerBore/django-rq#2 implements this solution, if you want to take a look...