WordPress / openverse

Openverse is a search engine for openly-licensed media. This monorepo includes all application code.
https://openverse.org
MIT License
240 stars 191 forks source link

Update the API to Django 5 #4015

Open sarayourfriend opened 5 months ago

sarayourfriend commented 5 months ago

Problem

Django 5 is out and the initial bugs are sorted. It has some significant QoL and other improvements for async routes, and we should update to Django 5.

Description

Review the Django 5 release notes and cross-reference breaking changes with our API codebase.

https://docs.djangoproject.com/en/5.0/releases/5.0/#backwards-incompatible-changes-in-5-0

At a glance, I don't think we are affected by the breaking changes, but please take a close look at each item to confirm this.

Update the API to Django 5 and make any necessary changes.

Remove any backported code that may exist to add Django 5 features before we've upgraded (e.g., cache_control backport for https://github.com/WordPress/openverse/issues/4005).

sarayourfriend commented 3 weeks ago

@WordPress/openverse-maintainers would be great to get this issue going soon. 4.2 is an LTS release and supported until 2026, so we are not at risk of falling out of support. However, as noted in the issue description, 5 and 5.1 already have loads of improvements we would benefit from, including and especially for async routes.

5.1, for example, finally has support for connection pooling Postgres: https://docs.djangoproject.com/en/5.1/releases/5.1/#postgresql-connection-pools

It also displays "operation categories" for migrations, making it easier to understand what the effect of a migration will be, potentially helping avoid unexpected types of migrations (like ones that might not be zero-downtime), without relying on careful, manual PR-time review of instructions: https://docs.djangoproject.com/en/5.1/ref/migration-operations/#django.db.migrations.operations.base.OperationCategory

5.1 has a few deprecated features, which may require changes after the fact to address. The main one we might need to address is deprecation of position arguments to Model.save() and asave(). We do not need to address those immediately upon upgrading, as they are only deprecated, not removed.

5.0 added support for ASGI disconnects, which would be nice to have in order to safely halt longer running requests in async views (e.g., aborting outbound aiohttp requests in the thumbnail route, killing audiowaveform processes in the waveform route, etc).

5.1 added support for ModelAdmin.list_display to target boolean attributes, and __ attributes for joining across models, allowing us to delete this code (and especially the comment explaining the one-liner), for example:

https://github.com/WordPress/openverse/blob/1ea37d77ce732c31d14dd78adaaeb8b96a7e56db/api/api/admin/media_report.py#L843-L853

There may be a few more things, but these were the ones that jumped out to me.

For these reasons, I will put this into the todo column.

I've read through the deprecations and removals once more (and for the first time for 5.1) and it still doesn't look like we will be affected, but our CI should tell us if I'm wrong. Luckily, our general usage of Django is actually pretty simple and small, so the surface area we have to worry about is not too big... but it will get bigger the longer we wait to upgrade!