-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Add uvicorn and web sockets for native Django 3 #2506
Conversation
* Add use_async option to cookiecutter.json * Add websocket configuration
Adding proper configuration. Not done. However, I'm looking for opinions regarding the necessity of this. |
* Performance is very low
* The reason there was a degraded performance was because we're using Gunicorn itself as a local tester. So we needed to add the staticfiles urls
Edit: it wasn't a security feature. It was because |
If you're wondering about the use-cases for web sockets, I've written a gist that talks about how to implement a one websocket per user system for passing information asynchronously. Basically, it can help with continuously feeding users information through a system of headers. To receive events from other clients, start an asyncio loop in the web sockets. Additionally, #2510 would come in handy. Although, that would need approval first if I were to implement it here. |
@browniebroke PR for ASGI is finished. Note for the future, we should leave an example of using |
I was puzzled as well at first when I saw no Personally, I would add the variable to |
@wadkar Currently, the uvicorn command includes the environment variable TL;DR It wasn't a security feature. It was more like cookiecutter didn't need to do it since |
Ah, I see. So correct me if I got it wrong, but this PR replaces My question is then what would be our recommendation for people developing locally without docker then? Should they rely on |
This PR enables ASGI/async work with native Django 3.0. It replaces
Really, the commands I have in the two To be fair though, you only needed to do use Uvicorn for local development, but I found it better if the mock server of Gunicorn + Uvicorn would be better for simulation. However, for production, you should use the G+U combo. |
Thanks a lot! This is super helpful. I am working on a branch with PostGIS support here. The branch works on top of this PR as my use case requires an async worker. |
@wadkar A little off topic, but you can look at my repo sample-user-location for how I set up geolocation with PostGIS. Just make sure you add DATABASES[“default”][“engine”] = “postgis” and replace postgis with the actual thing and you need to add the geolocation dependencies to the Dockerfile which’re listed in the geodjango installation docs. Additionally, be careful with async + Django ORM using psycopg2. Django ORM is NOT thread safe, and although you can use asgiref + Django channel’s database_sync_to_async method for db calling, I do not recommend it. I’d only recommend using a Celery task to perform it to be thread safe. |
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.
Amazing work @Andrew-Chen-Wang! I've finally found some time to give this a spin and it seems to work. I've used this JS snippet (from this post) to test it out:
> ws = new WebSocket('ws://localhost:8000/') // or 'wss://<mydomain.com>/' in prod
WebSocket {url: "ws://localhost:8000/", readyState: 0, bufferedAmount: 0, onopen: null, onerror: null, …}
> ws.onmessage = event => console.log(event.data)
event => console.log(event.data)
> ws.send("ping")
undefined
pong!
It's a shame that we need drop runserver_plus
locally, but we can always improve it later, we're still using it if use_async=n
anyway.
I've noted a couple of suggestions and something is missing for Heroku. Should we add a section about async to the Getting Up and Running Locally (without Docker) page?
* Deleted some unnecessary info inside asgi.py
@browniebroke Thanks for taking a look and testing it! I updated the docs for local development. I didn't really test the Gulp configuration since I'm not exactly sure how to use Gulpfile.js. Should I remove my edits to it? |
Good spot with Gulp, I will take a look to that part as well. |
That'd be helpful! @bahmdev Can you make a PR in my fork? Not exactly sure how to integrate it since it's probably gonna be a bit of fixing up LOGGING... |
@Andrew-Chen-Wang I can do that, but I might not get to it until later this week. |
This is to be consistent with `manage.py` behaviour and required to run locally.
Thanks a lot for this @Andrew-Chen-Wang 🎉 |
@bahmdev Any updates? Or any tips on how I could do it myself? I'm just not sure how to using Sentry. |
@Andrew-Chen-Wang Based on the documentation it seems all you need to do is add the sentry middleware after you get the application in import sentry_sdk
from sentry_sdk.integrations.asgi import SentryAsgiMiddleware
# Not sure about the environment part in local/dev
# but typically you are expected to have SENTRY_DSN declared in the environment
SENTRY_DSN = env("SENTRY_DSN")
sentry_sdk.init(dsn="SENTRY_DSN")
...
django_application = get_asgi_application()
django_application = SentryAsgiMiddleware(django_application) |
@wadkar It doesn't seem like it's even necessary. According to getsentry/python-sentry, it seems like the Django integration automatically looks for the handler (either WSGI or ASGI). I haven't tested it, but, since the connect method for websockets is essentially an HTTP request, I'm assuming it would work correctly for initial request. However, an internal error could be different. I'm pretty sure if you've got any kind of internal error, though, it would be caught by the Sentry middleware regardless. So I guess the above wouldn't be necessary. |
@Andrew-Chen-Wang great work man! |
Description
Add uvicorn to Docker with Django async capabilities
Optional based on cookiecutter.json
Rationale
Django is pushing for async especially with the thought of psycopg3 becoming async too to make connecting to the database thread safe. Thus, we should, too.
Use case(s) / visualization(s)
Most apps are going to need async at some point whether it be through chat applications (which you can easily do using web sockets and celery + redis already installed as a message queue + broker).
SLIGHTLY Fixes #2314. I refuse to add Django-channels because Django itself is moving towards thread-safe database access. And even if you want to create a chat app, you can simply use celery + redis, which is already added via docker if you turn on that configuration, and create objects through celery tasks. Django-channels creates a new thread; too many threads and you're gonna destroy your server. With a celery and native Django 3.0 based chat, you need not create that many threads since you're limited by memory in this case. It works better if you also use
bulk_create
.Cookiecutter-django can now support ASGI which allows for HTTP/2, meaning faster speeds.