-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Feature/django channels chat app #2228
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
base: main
Are you sure you want to change the base?
Feature/django channels chat app #2228
Conversation
@tim-schilling I wasn't entirely sure whether to include the tests for the example app itself, seeing as it is something for testing, rather than a change to the Debug Toolbar itself. I was also unsure of the location to add the test. |
@Chiemezuo I think we should add it to give the users of the example app that the code in there works as expected |
@JohananOppongAmoateng I think that's an excellent idea. The test requires |
path("async/chat/", async_chat_index, name="async_chat"), | ||
path("async/chat/<str:room_name>/", async_chat_room, name="room"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these need go to in async_/urls.py
"websocket": AllowedHostsOriginValidator( | ||
AuthMiddlewareStack(URLRouter(websocket_urlpatterns)) | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is why the toolbar doesn't work with the channels' consumers. When you send a chat message via the websocket, that message isn't visible in the toolbar anywhere. Only the HTTP requests are. It bypasses the middleware that the toolbar works with.
To start, we would need to define a another middleware that is a subclass of channels.middleware.BaseMiddleware
. There may be other issues that arise after that. However, I think this is enough for us to declare the toolbar doesn't support Django Channels' consumers yet 😢
CHANNEL_LAYERS = { | ||
"default": { | ||
"BACKEND": "channels_redis.core.RedisChannelLayer", | ||
"CONFIG": { | ||
"hosts": [("127.0.0.1", 6379)], | ||
}, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably go in async_/settings.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for doing all this! It really helps me see why it doesn't work and where the changes need to go.
So we have a choice to make. We can integrate this proactively, or we can store put this on an issue to make use of in the future when we ship a properly working channels consumer integration. If we don't have immediate plans to work on it, then I would vote for putting all the code in an issue and/or reference this branch so another can continue. However, if someone is planning on working on it in the next few months, then I'm good with merging it.
We will need to update the Django Channels & Async
section of the docs at a minimum. It should mention that the toolbar will not work with any Consumer views since it bypasses the Django middleware for the ASGI application.
@@ -0,0 +1,27 @@ | |||
<!-- example/templates/chat/index.html --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<!-- example/templates/chat/index.html --> | |
{% comment %} | |
This template is from the Django Channels "Web Socket" chat app | |
https://channels.readthedocs.io/en/latest/tutorial/index.html | |
{% endcomment %} |
@@ -0,0 +1,49 @@ | |||
<!-- example/templates/chat/room.html --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<!-- example/templates/chat/room.html --> | |
{% comment %} | |
This template is from the Django Channels "Web Socket" chat app | |
https://channels.readthedocs.io/en/latest/tutorial/index.html | |
{% endcomment %} |
@@ -0,0 +1,36 @@ | |||
# This is the consumer logic for the Django Channels "Web Socket" chat app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# This is the consumer logic for the Django Channels "Web Socket" chat app | |
# This is the consumer logic for the Django Channels "Web Socket" chat app | |
# https://channels.readthedocs.io/en/latest/tutorial/index.html |
@@ -0,0 +1,8 @@ | |||
# This is the routing logic for the Django Channels "Web Socket" chat app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# This is the routing logic for the Django Channels "Web Socket" chat app | |
# This is the routing logic for the Django Channels "Web Socket" chat app | |
# https://channels.readthedocs.io/en/latest/tutorial/index.html |
<li><a href="{% url 'turbo' %}">Hotwire Turbo</a></li> | ||
<li><a href="{% url 'htmx' %}">htmx</a></li> | ||
<li><a href="{% url 'bad_form' %}">Bad form</a></li> | ||
<li><a href="{% url 'async_chat' %}">Chat app</a></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the URLs will only be used for the async version, this probably will need to move down into the commented out area near async_db_view
. It's not great, but I don't have a better solution at the moment.
Description
This PR adds the Django Channels' demo Chat app into the Debug Toolbar's example app for easy testing. This is in line to support async and show the Debug Toolbar's compatibility with Django Channels
Fixes #2202
Checklist:
docs/changes.rst
.