Fix #672 #687
Fix #672 #687
Conversation
|
Thanks for the PR. Mind fixing the failed type lint? |
|
Hey @selwin you are welcome. I saw the warning but this isn't something related to this PR: The mypy warning is on https://github.com/rq/django-rq/blob/master/django_rq/management/commands/rqworker-pool.py#L97 while I only changed a totally different file (utils.py) without messing with types at all: https://github.com/rq/django-rq/pull/687/files |
|
Hello @selwin could you please give some love to this issue? It's a very, very small change but the dashboard isn't working at all for me without it :( TIA! |
| @@ -111,7 +111,7 @@ def get_scheduler_statistics(): | |||
| connection = first_sentinel.connection_pool.connection_kwargs | |||
| else: | |||
| connection = queue.connection.connection_pool.connection_kwargs | |||
| conn_key = f"{connection['host']}:{connection.get('port', 6379)}/{connection.get('db', 0)}" | |||
| conn_key = f"{connection.get('host', 'NOHOST')}:{connection.get('port', 6379)}/{connection.get('db', 0)}" | |||
There was a problem hiding this comment.
Hey, I'm not sure why falling back to NOHOST would fix this issue. Care to elaborate on why the Redis connection has no host property?
There was a problem hiding this comment.
Yes of course! First of all this is only for the name of a string key, ie it will just use "NOHOST" instead of the host of the connection. We could display anything we wanted here or even leave it blank, i.e do connection.get('host').
Second, to understand the probem, I am using a unix connection for my redis, something like:
RQ_QUEUES = {
"default": {
"URL": "unix:///home/serafeim/example/redis.sock",
"DB": 0,
"DEFAULT_TIMEOUT": 360,
}
}So there's no 'host' key on my connection dict resulting in an exception when calling connection['host'] !
|
Hello friends, a kind reminder for taking a look at the small fix for the issue, thank you! |
|
Hi, could you please fix the type check? |
|
Hey @selwin I can't really fix it because the error isn't related to my changes, please see the explanation in this comment #687 (comment) TIA |
|
Newer pull requests (e.g #693 ) don’t seem to have this issue. Do you mind pulling the master branch into your PR to see if that fixes the issue? |
|
@selwin I updated my pr with the django-rq master but now there's another error about id! Please notice that the django-rq master also has that error so it was expected that this would happen .. |
No description provided.