Skip to content
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

fix(views): skip checks if value isn't iterable #168

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

alpden550
Copy link

Fix TypeError at /watchman/dashboard/

for dashboard view, when Redis or something else is down

@mwarkentin
Copy link
Owner

@alpden550 thanks for the PR, just wanted to let you know that I'm on vacation through august so won't have a chance to merge / release this for a while.

@alpden550
Copy link
Author

@mwarkentin ok, thank you!

@mwarkentin mwarkentin self-assigned this Oct 21, 2021
@mwarkentin
Copy link
Owner

@alpden550 really sorry about not getting back to this after I came back online - I'm working on pulling some things together for a new releases and was wondering if you're still interested in getting this change in? It would be helpful if you'd be available to test a preview release or answer any questions I run into. Let me know! :)

@alpden550
Copy link
Author

@mwarkentin
Hello, what exactly do you mean about answering any questions ?

@mwarkentin
Copy link
Owner

@alpden550 just in case I have some trouble trying to reproduce / test the fix, etc.

@alpden550
Copy link
Author

@mwarkentin of course
I've caught it when not Redis or celery exists and instance was not iterable

@mwarkentin
Copy link
Owner

@alpden550 I'm trying to reproduce the issue on main so that I can test this fix..

I'm testing on the sample project which can be run in docker using make run.

I added a Redis cache backend, but am not running Redis:

❯ g diff
diff --git a/sample_project/requirements.txt b/sample_project/requirements.txt
index c60fab3..c7472e0 100644
--- a/sample_project/requirements.txt
+++ b/sample_project/requirements.txt
@@ -1,4 +1,5 @@
 django<5.0,>=4.0
+redis==4.1.0

 # django-watchman
 -e /app
diff --git a/sample_project/sample_project/settings.py b/sample_project/sample_project/settings.py
index b4438d6..e257a4c 100644
--- a/sample_project/sample_project/settings.py
+++ b/sample_project/sample_project/settings.py
@@ -108,6 +108,13 @@ AUTH_PASSWORD_VALIDATORS = [
     },
 ]

+CACHES = {
+    'default': {
+        'BACKEND': 'django.core.cache.backends.redis.RedisCache',
+        'LOCATION': 'redis://127.0.0.1:6379',
+    }
+}
+

When I hit the dashboard I see:
image

And the traceback:

Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/redis/connection.py", line 607, in connect
    sock = self._connect()
  File "/usr/local/lib/python3.10/site-packages/redis/connection.py", line 671, in _connect
    raise err
  File "/usr/local/lib/python3.10/site-packages/redis/connection.py", line 659, in _connect
    sock.connect(socket_address)
ConnectionRefusedError: [Errno 111] Connection refused

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/app/watchman/decorators.py", line 29, in wrapped
    response = func(*args, **kwargs)
  File "/app/watchman/checks.py", line 29, in _check_cache
    cache.set(key, value)
  File "/usr/local/lib/python3.10/site-packages/django/core/cache/backends/redis.py", line 191, in set
    self._cache.set(key, value, self.get_backend_timeout(timeout))
  File "/usr/local/lib/python3.10/site-packages/django/core/cache/backends/redis.py", line 108, in set
    client.set(key, value, ex=timeout)
  File "/usr/local/lib/python3.10/site-packages/redis/commands/core.py", line 1691, in set
    return self.execute_command("SET", *pieces, **options)
  File "/usr/local/lib/python3.10/site-packages/redis/client.py", line 1170, in execute_command
    conn = self.connection or pool.get_connection(command_name, **options)
  File "/usr/local/lib/python3.10/site-packages/redis/connection.py", line 1311, in get_connection
    connection.connect()
  File "/usr/local/lib/python3.10/site-packages/redis/connection.py", line 611, in connect
    raise ConnectionError(self._error_message(e))
redis.exceptions.ConnectionError: Error 111 connecting to 127.0.0.1:6379. Connection refused.

Any chance you could provide an example config which can help me reproduce this issue?

@alpden550
Copy link
Author

@mwarkentin yes, I got it and reproduce for you.

I use docker-compose containers, and if all services work, all fine. Example:

Снимок экрана 2022-01-16 в 09 09 24

My services like that, nothing specials

version: '3.9'

services:
  app:
    build: .
    command: python manage.py runserver 0.0.0.0:8000
    volumes:
      - .:/app
    ports:
      - "8000:8000"
    env_file:
      - url_shortener/.env.dev
    depends_on:
      - db
      - redis

  db:
    image: postgres
    env_file:
      - url_shortener/.env.dev

  redis:
    image: redis

  celery:
    build: .
    volumes:
      - .:/app
    depends_on:
      - redis
      - db
    env_file:
      - url_shortener/.env.dev
    command: celery -A url_shortener worker -l info

  email:
    build: .
    volumes:
      - .:/app
    depends_on:
      - redis
      - db
    env_file:
      - url_shortener/.env.dev
    command: celery -A url_shortener worker -l info

  tgbot:
    build: .
    env_file:
      - url_shortener/.env.dev
    depends_on:
      - app
    links:
      - app
    command: sh -c "python manage.py startbot"

But if Redis was stopped, this error happens

изображение

@mwarkentin
Copy link
Owner

Thanks, I'll give it another try to reproduce with what you've provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants