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

Add uvicorn and web sockets for native Django 3 #2506

Merged
merged 17 commits into from
Apr 16, 2020

Conversation

Andrew-Chen-Wang
Copy link
Contributor

@Andrew-Chen-Wang Andrew-Chen-Wang commented Mar 24, 2020

Description

Add uvicorn to Docker with Django async capabilities

Optional based on cookiecutter.json

  • Add use_async option to cookiecutter.json
  • Add websocket configuration

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.

@Andrew-Chen-Wang
Copy link
Contributor Author

Andrew-Chen-Wang commented Mar 24, 2020

Adding proper configuration. Not done. However, I'm looking for opinions regarding the necessity of this.

* 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
@Andrew-Chen-Wang
Copy link
Contributor Author

Andrew-Chen-Wang commented Mar 25, 2020

@browniebroke It should work work now. Heed one thing though: you can't run the server without setting an environment variable DJANGO_SETTINGS_MODULE=config.settings.local in the uvicorn command. I also noticed it wasn't in the .django env file (is this a security feature?).

Edit: it wasn't a security feature. It was because runserver doesn't use wsgi.py and uses its mock server instead since when you deploy, you use wsgi.py where the DJANGO_SETTINGS_MODULE env variable has a new default of the production settings.

@Andrew-Chen-Wang
Copy link
Contributor Author

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.

@Andrew-Chen-Wang
Copy link
Contributor Author

@browniebroke PR for ASGI is finished.

Note for the future, we should leave an example of using asgiref.sync.sync_to_async and Django channel's channels.db.database_sync_to_async for other users. Additionally, I found it more useful to have websocket.py inside of an app instead of the config file. For me, I prefer using sync_to_async with Celery and for chat apps I use Redis Pub/Sub with a connection singleton per Django.

@wadkar
Copy link
Contributor

wadkar commented Apr 10, 2020

@browniebroke It should work work now. Heed one thing though: you can't run the server without setting an environment variable DJANGO_SETTINGS_MODULE=config.settings.local in the uvicorn command. I also noticed it wasn't in the .django env file (is this a security feature?).

I was puzzled as well at first when I saw no DJANGO_SETTINGS_MODULE in ./envs/.{local,production}/.django and decided to add it explicitly to my project. Would like to know the rationale/usage or a pointer to the wiki/issue/PR where this was discussed/introduced.

Personally, I would add the variable to ./envs/.{local,production}/.django instead of "hardcoding" it in ../local/start script file because I believe it is the file to store environment variables.

@Andrew-Chen-Wang
Copy link
Contributor Author

Andrew-Chen-Wang commented Apr 10, 2020

@wadkar Currently, the uvicorn command includes the environment variable DJANGO_SETTINGS_MODULE=config.settings.local so if you run docker-compose -f local.yml up, it'll default to the local settings. I'm pretty sure the reason the variable is NOT in the .envs is because the current start file for Django uses the runserver which SKIPS over the wsgi.py and uses an internal mock server since the wsgi.py re-sets the config to production https://github.com/Andrew-Chen-Wang/cookiecutter-django/blob/async/%7B%7Bcookiecutter.project_slug%7D%7D/config/wsgi.py#L30. When you run gunicorn in production (let's say without this PR), in the wsgi.py, the settings are re-set to a default of the production settings.

TL;DR It wasn't a security feature. It was more like cookiecutter didn't need to do it since runserver never actually uses wsgi.py.

@Andrew-Chen-Wang Andrew-Chen-Wang changed the title Add uvicorn and web sockets for Django 3 Add uvicorn and web sockets for native Django 3 Apr 10, 2020
@wadkar
Copy link
Contributor

wadkar commented Apr 11, 2020

TL;DR It wasn't a security feature. It was more like cookiecutter didn't need to do it since runserver never actually uses wsgi.py.

Ah, I see. So correct me if I got it wrong, but this PR replaces runserver with gunicorn inside docker for local environment and thus needs to be passed the -e DJANGO_SETTING_MODULE=settings.config.local explicitly.

My question is then what would be our recommendation for people developing locally without docker then? Should they rely on runserver.py for this? Will runserver.py allow the devs to do async development?

@Andrew-Chen-Wang
Copy link
Contributor Author

This PR enables ASGI/async work with native Django 3.0. It replaces python manage.py runserver because the runserver command creates a mock synchronous server. In order to run asynchronously, I had to use Uvicorn. In cookiecutter.json, I added the option "use_async" if you wanted the async files and commands, so I didn't ENTIRELY replaces the runserver command; it's just not used if you want to use async. If you want to run locally, you can use the following command:

/usr/local/bin/gunicorn config.asgi --bind 0.0.0.0:8000 --chdir=/app -k uvicorn.workers.UvicornWorker -e DJANGO_SETTINGS_MODULE=config.settings.local --reload

Really, the commands I have in the two start files are the commands you should probably use for your local and production settings. Again, run server cannot do async development, as noted in Django docs, so I had to substitute it with running our own server, i.e. Uvicorn.

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.

wadkar referenced this pull request in wadkar/cookiecutter-django Apr 11, 2020
@wadkar
Copy link
Contributor

wadkar commented Apr 11, 2020

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.

@Andrew-Chen-Wang
Copy link
Contributor Author

@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.

Copy link
Member

@browniebroke browniebroke left a 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?

{{cookiecutter.project_slug}}/config/asgi.py Outdated Show resolved Hide resolved
{{cookiecutter.project_slug}}/config/asgi.py Outdated Show resolved Hide resolved
@Andrew-Chen-Wang
Copy link
Contributor Author

@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?

@browniebroke
Copy link
Member

Good spot with Gulp, I will take a look to that part as well.

@Andrew-Chen-Wang
Copy link
Contributor Author

Andrew-Chen-Wang commented Apr 13, 2020

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...

@bahmdev
Copy link

bahmdev commented Apr 15, 2020

@Andrew-Chen-Wang I can do that, but I might not get to it until later this week.

@browniebroke browniebroke merged commit 4e789d8 into cookiecutter:master Apr 16, 2020
@browniebroke
Copy link
Member

Thanks a lot for this @Andrew-Chen-Wang 🎉

@Andrew-Chen-Wang
Copy link
Contributor Author

@bahmdev Any updates? Or any tips on how I could do it myself? I'm just not sure how to using Sentry.

@wadkar
Copy link
Contributor

wadkar commented Apr 22, 2020

@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 asgi.py file:

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)

@Andrew-Chen-Wang
Copy link
Contributor Author

@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.

@stefanitsky
Copy link
Contributor

stefanitsky commented May 25, 2020

@Andrew-Chen-Wang great work man!

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

Successfully merging this pull request may close these issues.

add uvicorn with gunicorn and django channels
5 participants