Skip to content

Remove async #395

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

Merged
merged 8 commits into from
Apr 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ exclude: "^docs/|.idea|devcontainer.json|tests/fixtures|tests/.*snapshots/.*\\.(
default_stages: [pre-commit]

default_language_version:
python: python3.12
python: python3.13

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
Expand Down Expand Up @@ -64,7 +64,7 @@ repos:

- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.9.10
rev: v0.11.2
hooks:
- id: ruff
args: [--fix, --preview]
Expand Down
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@

## Unreleased

## 25.04.1

- Remove async functions to simplify the code.
- We now run with `gunicorn` instead of `daphne` in production. We use 4 `gunicorn` workers.
- Switch to Python 3.13
- Correct title for feeds without sections in feeds admin.

## 25.03.2

- Improve user Django admin page.
- Prevent errors with empty slugs.

## 25.03.1

- Can filter tags in tags admin.
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ You will have to start Django with the provided run target.

### Using Pycharm

By default, everything is set up to develop locally with Pycharm. So you will need docker (for the database), [uv](https://docs.astral.sh/uv/), Python 3.12 and nodeJS 20+ installed for this to work.
By default, everything is set up to develop locally with Pycharm. So you will need docker (for the database), [uv](https://docs.astral.sh/uv/), Python 3.13 and nodeJS 20+ installed for this to work.
Django will be started automatically.
On the first run, you must run `npm install` to install a few JS deps and `uv run pre-commit install --hook-type pre-commit --hook-type pre-push` to configure `pre-commit`.

Expand Down
61 changes: 30 additions & 31 deletions config/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import asgiref
import django
import environ
from csp.constants import NONCE, NONE, SELF, STRICT_DYNAMIC, UNSAFE_INLINE
from django.contrib.messages import constants as messages
from django.utils.translation import gettext_lazy as _
from template_partials.apps import wrap_loaders
Expand Down Expand Up @@ -101,7 +102,6 @@
# APPS
# ------------------------------------------------------------------------------
DJANGO_APPS = [
"daphne",
"django.contrib.auth",
"django.contrib.contenttypes",
"django.contrib.sessions",
Expand Down Expand Up @@ -332,30 +332,31 @@
# https://django-csp.readthedocs.io/en/latest/configuration.html
# https://content-security-policy.com/
# https://csp-evaluator.withgoogle.com/
CSP_DEFAULT_SRC = ("'self'",)
CSP_SCRIPT_SRC = ("'strict-dynamic'", "'unsafe-inline'", "https:")
CSP_SCRIPT_SRC_ATTR = None
CSP_SCRIPT_SRC_ELEM = None
CSP_IMG_SRC = ("'self'", "data:")
CSP_OBJECT_SRC = ("'none'",)
CSP_MEDIA_SRC = ("'self'",)
CSP_FRAME_SRC = ("'none'",)
CSP_FONT_SRC = ("'self'",)
CSP_CONNECT_SRC = env.tuple("CSP_CONNECT_SRC", default=("'self'",))
CSP_STYLE_SRC = ("'strict-dynamic'", "'unsafe-inline'", "https:")
CSP_STYLE_SRC_ATTR = None
CSP_STYLE_SRC_ELEM = None
CSP_BASE_URI = ("'none'",)
CSP_FRAME_ANCESTORS = ("'none'",)
CSP_FORM_ACTION = ("'self'",)
CSP_MANIFEST_SRC = ("'self'",)
CSP_WORKER_SRC = ("'self'",)
CSP_PLUGIN_TYPES = None
CSP_REQUIRE_SRI_FOR = None
CSP_INCLUDE_NONCE_IN = ("script-src", "style-src")
# Those are forced to true in production
CSP_UPGRADE_INSECURE_REQUESTS = IS_PRODUCTION
CSP_BLOCK_ALL_MIXED_CONTENT = IS_PRODUCTION
CONTENT_SECURITY_POLICY = {
"DIRECTIVES": {
"default-src": (SELF,),
"script-src": (STRICT_DYNAMIC, UNSAFE_INLINE, "https:", NONCE),
"script-src-attr": None,
"script-src-elem": None,
"img-src": (SELF, "data:"),
"object-src": (NONE,),
"media-src": (SELF,),
"frame-src": (NONE,),
"font-src": (SELF,),
"connect-src": env.tuple("CSP_CONNECT_SRC", default=(SELF,)),
"style-src": (STRICT_DYNAMIC, UNSAFE_INLINE, "https:", NONCE),
"style-src-attr": None,
"style-src-elem": None,
"base-uri": (NONE,),
"child-src": (NONE,),
"frame-ancestors": (NONE,),
"form-action": (SELF,),
"manifest-src": (SELF,),
"worker-src": (SELF,),
"require-sri-for": (NONE,),
"upgrade-insecure-requests": IS_PRODUCTION,
},
}


# CORS
Expand Down Expand Up @@ -490,11 +491,9 @@
# ------------------------------------------------------------------------------
ACCOUNT_ALLOW_REGISTRATION = env.bool("DJANGO_ACCOUNT_ALLOW_REGISTRATION", True)
# https://django-allauth.readthedocs.io/en/latest/configuration.html
ACCOUNT_AUTHENTICATION_METHOD = "email"
# https://django-allauth.readthedocs.io/en/latest/configuration.html
ACCOUNT_EMAIL_REQUIRED = True
ACCOUNT_LOGIN_METHODS = {"email"}
# https://django-allauth.readthedocs.io/en/latest/configuration.html
ACCOUNT_USERNAME_REQUIRED = False
ACCOUNT_SIGNUP_FIELDS = ["email*", "password1*", "password2*"]
# https://django-allauth.readthedocs.io/en/latest/configuration.html
ACCOUNT_USER_MODEL_USERNAME_FIELD = None
# https://django-allauth.readthedocs.io/en/latest/configuration.html
Expand All @@ -515,7 +514,7 @@
# django-version-checks (https://pypi.org/project/django-version-checks/)
# ------------------------------------------------------------------------------
VERSION_CHECKS = {
"python": "==3.12.*",
"python": "==3.13.*",
"postgresql": ">=16",
}

Expand Down Expand Up @@ -623,7 +622,7 @@ def before_send_to_sentry(event, hint):
# ------------------------------------------------------------------------------
# See https://django-ninja.dev/reference/settings/
NINJA_PAGINATION_MAX_LIMIT = 500
NINJA_PAGINATION_CLASS = "legadilo.utils.pagination.LimitOffsetPagination"
NINJA_PAGINATION_CLASS = "ninja.pagination.LimitOffsetPagination"


# Legadilo's specific stuff...
Expand Down
2 changes: 1 addition & 1 deletion devops/compose/local/django/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM python:3.12-slim-bookworm
FROM python:3.13-slim-bookworm

ARG BUILD_ENVIRONMENT=local
ARG APP_HOME=/app
Expand Down
2 changes: 1 addition & 1 deletion devops/compose/local/docs/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM python:3.12-slim-bookworm
FROM python:3.13-slim-bookworm

ARG BUILD_ENVIRONMENT
ENV PYTHONUNBUFFERED 1
Expand Down
4 changes: 2 additions & 2 deletions devops/compose/production/django/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ COPY ./package-lock.json /

RUN cd / && npm install

FROM python:3.12-slim-bookworm AS python-builder
FROM python:3.13-slim-bookworm AS python-builder

ARG BUILD_ENVIRONMENT=production
ARG APP_HOME=/app
Expand Down Expand Up @@ -46,7 +46,7 @@ RUN chmod +x /setup-django.sh
RUN /setup-django.sh


FROM python:3.12-slim-bookworm
FROM python:3.13-slim-bookworm

ARG BUILD_ENVIRONMENT=production
ARG APP_HOME=/app
Expand Down
9 changes: 6 additions & 3 deletions devops/compose/production/django/start.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,22 @@ set -o errexit
set -o pipefail
set -o nounset

readonly SERVER_PORT=8000
readonly NB_GUNICORN_WORKERS=4
readonly GUNICORN_REQUEST_TIMEOUT=60

python manage.py createcachetable
python manage.py migrate

cmd="${1:-server}"

case "${cmd}" in
server)
SERVER_PORT=8000
echo "Starting server on port ${SERVER_PORT}"
if [[ "${BUILD_ENV}" == "local" ]]; then
exec python manage.py runserver 0.0.0.0:${SERVER_PORT}
exec python manage.py runserver "0.0.0.0:${SERVER_PORT}"
else
exec daphne config.asgi:application --bind 0.0.0.0 --port ${SERVER_PORT} --no-server-name --verbosity 1
exec gunicorn config.wsgi:application --bind "0.0.0.0:${SERVER_PORT}" --timeout ${GUNICORN_REQUEST_TIMEOUT} --workers ${NB_GUNICORN_WORKERS}
fi
;;
cron)
Expand Down
35 changes: 35 additions & 0 deletions docs/adrs/0009-remove-asnyc-for-now.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# 9 - Remove async for now

* **Date:** 2025-04-01
* **Status:** Accepted
* Partially supersedes <project:./0000-project-setup.md> and <project:./0005-deployment.md>

## Context

After using async, it brings complexity for performance improvements we need yet.
The main issue being that we cannot yet use transactions in an async context (see [this issue](https://code.djangoproject.com/ticket/33882)).
This results in extra complexities with usages of `sync_to_async` and `async_to_sync` to switch from one context to another.

Since we do multiple database operations, I think transaction must be used to always be in a valid state.
Or to state it differently: since we are far from performance problems yet, it’s best to just use transactions for consistency than to remove them to have cleaner async support or have the complexity that comes with sync to async conversions.

Since we do lots of async requests, I still think async makes sense.
Just not now.


## Decisions

Simply the code as much as we can and remove async support for now.
It will impact many files, but it should be relatively straightforward.


## Consequences

- We may have to re-add async later on.
- Lots of code changes.
- Must have an export command to export big files.
- Use `gunicorn` to run the application in production:
- It’s kind of a default choice for this, and I’ve used it in the past with great success.
- Create an incoming HTTP request timeout. Set it to 60s to allow for slow search.
- Use 4 workers: from my experience, it’s a good number.
- We may need to allow for incoming request timeout and number of workers to be configured.
2 changes: 1 addition & 1 deletion justfile
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ release:
echo "Creating version ${new_tag} Press enter to accept."
read -r

{{docker-cmd}} pull python:3.12-slim-bookworm
{{docker-cmd}} pull python:3.13-slim-bookworm
{{docker-compose-cmd}} -f production.yml build --build-arg "VERSION=${new_tag}" django
{{docker-cmd}} image tag legadilo_production_django:latest "rg.fr-par.scw.cloud/legadilo/legadilo-django:${new_tag}"
{{docker-cmd}} image tag legadilo_production_django:latest rg.fr-par.scw.cloud/legadilo/legadilo-django:latest
Expand Down
1 change: 1 addition & 0 deletions legadilo/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.

SEARCHED_TEXT_MIN_LENGTH = 3
MAX_PARALLEL_CONNECTIONS = 10
Loading