Opened 6 years ago
Closed 6 years ago
#31380 closed New feature (fixed)
Deploy system check that DJANGO_ALLOW_ASYNC_UNSAFE is not set
| Reported by: | Adam Johnson | Owned by: | hashlash |
|---|---|---|---|
| Component: | Core (System checks) | Version: | 3.0 |
| Severity: | Normal | Keywords: | |
| Cc: | Andrew Godwin | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description
Django's async-safety feature disable itself if the environment variable DJANGO_ALLOW_ASYNC_UNSAFE is set to any value. This is useful for jupyter notebooks and similar, however it can lead to data loss in production.
I suggest a deploy-tagged system check that returns an error if it is set, to guard against it being used for production web traffic.
Attachments (1)
Change History (14)
comment:1 by , 6 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:3 by , 6 years ago
Anyone picking this up see previous system checks PR's for all the places to edit: https://github.com/django/django/pulls?q=is%3Apr+added+check+is%3Aclosed
by , 6 years ago
| Attachment: | async-unsafe.diff added |
|---|
Check DJANGO_ALLOW_ASYNC_UNSAFE on deployment implementation
follow-up: 5 comment:4 by , 6 years ago
@hashlash please make a PR on GitHub
Also I think the tag should be 'async', if we even need to add a tag.
Please also add documentation and release note.
comment:5 by , 6 years ago
Replying to Adam (Chainz) Johnson:
Also I think the tag should be 'async', if we even need to add a tag.
Something like this?
class Tags:
...
async = 'async'
...
But async is a reserved word in Python. Or do you mean the string part only and leave the attribute as asyncio?
class Tags:
...
asyncio = 'async'
...
follow-up: 7 comment:6 by , 6 years ago
Forgot it's a reserved word.
I think we want to avoid 'asyncio' since that's a specific library, and in theory async stuff can move to other async libraries like curio / trio. Maybe 'asgi' is a better tag, since that's the target framework for django async stuff. Or we could use async_ = 'async' as the tag, acknowledging it's a reserved word.
comment:7 by , 6 years ago
| Has patch: | set |
|---|---|
| Needs documentation: | set |
| Owner: | changed from to |
| Status: | new → assigned |
Replying to Adam (Chainz) Johnson:
I think we want to avoid 'asyncio' since that's a specific library, and in theory async stuff can move to other async libraries like curio / trio. Maybe 'asgi' is a better tag, since that's the target framework for django async stuff. Or we could use
async_ = 'async'as the tag, acknowledging it's a reserved word.
I'll take asgi and will make the PR. Tell if anybody has any thoughts about the naming
comment:8 by , 6 years ago
PR: https://github.com/django/django/pull/12596
Haven't submitted the documentation and release note yet. Will update soon
comment:10 by , 6 years ago
| Needs documentation: | set |
|---|
There's just a couple of docs comments on the PR. (Then this looks good to go.)
@hashlash: Please uncheck Needs documentation when done, to make sure we see it quickly.
Thanks!
comment:12 by , 6 years ago
| Component: | Utilities → Core (System checks) |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
OK, a deployment (
--deploy) seems reasonable.