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

Upgrade Django version to 5.2 #1900

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ repos:
rev: "1.22.2"
hooks:
- id: django-upgrade
# TODO: Update to 5.2 when django-upgrade supports it
args: [--target-version, "5.1"]
- repo: https://github.com/psf/black
rev: 24.10.0
Expand Down
1 change: 0 additions & 1 deletion djangoproject/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@
SESSION_COOKIE_HTTPONLY = True

SILENCED_SYSTEM_CHECKS = [
"fields.W342", # tracdb has ForeignKey(unique=True) in lieu of multi-col PKs
"security.W008", # SSL redirect is handled by nginx
"security.W009", # SECRET_KEY is setup through Ansible secrets
]
Expand Down
2 changes: 1 addition & 1 deletion requirements/common.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ django-push @ git+https://github.com/brutasse/django-push.git@22fda99641cfbd2f30
django-read-only==1.18.0
django-recaptcha==4.0.0
django-registration-redux==2.13
Django==5.1.5
Django==5.2a1
docutils==0.21.2
feedparser==6.0.11
Jinja2==3.1.5
Expand Down
46 changes: 6 additions & 40 deletions tracdb/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,38 +9,6 @@
* All the session and permission tables: they're just not needed.
* Enum: I don't know what this is or what it's for.
* NodeChange: Ditto.

These models are far from perfect but are Good Enough(tm) to get some useful data out.

One important mismatch between these models and the Trac database has to do with
composite primary keys. Trac uses them for several tables, but Django does not support
them yet (ticket #373).

These are the tables that use them:

* ticket_custom (model TicketCustom)
* ticket_change (model TicketChange)
* wiki (model Wiki)
* attachment (model Attachment)

To make these work with Django (for some definition of the word "work") we mark only
one of their field as being the primary key (primary_key=True).
This is obviously incorrect but — somewhat suprisingly — it doesn't break **everything**
and the little that does actually work is good enough for what we're trying to do:

* Model.objects.create(...) correctly creates the object in the db
* Most queryset/manager methods work (in particular filter(), exclude(), all()
and count())

On the other hand, here's what absolutely DOES NOT work (the list is sadly not
exhaustive):

* Updating a model instance with save() will update ALL ROWS that happen to share
the value for the field used as the "fake" primary key if they exist (resulting
in a DBError)
* The admin won't work (the "pk" field shortcut can't be used reliably since it can
return multiple rows)

"""

from datetime import date
Expand Down Expand Up @@ -161,11 +129,11 @@ def __str__(self):


class TicketCustom(models.Model):
pk = models.CompositePrimaryKey("ticket", "name")
ticket = models.ForeignKey(
Ticket,
related_name="custom_fields",
db_column="ticket",
primary_key=True, # XXX See note at the top about composite pk
on_delete=models.DO_NOTHING,
)
name = models.TextField()
Expand All @@ -180,11 +148,11 @@ def __str__(self):


class TicketChange(models.Model):
pk = models.CompositePrimaryKey("ticket", "_time", "field")
ticket = models.ForeignKey(
Ticket,
related_name="changes",
db_column="ticket",
primary_key=True, # XXX See note at the top about composite pk
on_delete=models.DO_NOTHING,
)
author = models.TextField()
Expand Down Expand Up @@ -292,9 +260,8 @@ def __str__(self):


class Wiki(models.Model):
name = models.TextField(
primary_key=True
) # XXX See note at the top about composite pk
pk = models.CompositePrimaryKey("name", "version")
name = models.TextField()
version = models.IntegerField()
_time = models.BigIntegerField(db_column="time")
time = time_property("_time")
Expand All @@ -312,10 +279,9 @@ def __str__(self):


class Attachment(models.Model):
pk = models.CompositePrimaryKey("type", "id", "filename")
type = models.TextField()
id = models.TextField(
primary_key=True
) # XXX See note at the top about composite pk
id = models.TextField()
filename = models.TextField()
size = models.IntegerField()
_time = models.BigIntegerField(db_column="time")
Expand Down
59 changes: 2 additions & 57 deletions tracdb/testutils.py
Original file line number Diff line number Diff line change
@@ -1,60 +1,5 @@
from copy import deepcopy

from django.apps import apps
from django.db import connections, models

# There's more models with a fake composite pk, but we don't test them at the moment.
_MODELS_WITH_FAKE_COMPOSITE_PK = {"ticketcustom"}


def _get_pk_field(model):
"""
Return the pk field for the given model.
Raise ValueError if none or more than one is found.
"""
pks = [field for field in model._meta.get_fields() if field.primary_key]
if len(pks) == 0:
raise ValueError(f"No primary key field found for model {model._meta.label}")
elif len(pks) > 1:
raise ValueError(
f"Found more than one primary key field for model {model._meta.label}"
)
else:
return pks[0]


def _replace_primary_key_field_with_autofield(model, schema_editor):
"""
See section about composite pks in the docstring for models.py to get some context
for this.

In short, some models define a field as `primary_key=True` but that field is not
actually a primary key. In particular that field is not supposed to be unique, which
interferes with our tests.

For those models, we remove the `primary_key` flag from the field, and we add a
new `testid` autofield. This makes the models easier to manipulate in the tests.
"""
old_pk_field = _get_pk_field(model)
del old_pk_field.unique
new_pk_field = deepcopy(old_pk_field)
new_pk_field.primary_key = False
schema_editor.alter_field(
model=model, old_field=old_pk_field, new_field=new_pk_field
)

autofield = models.AutoField(primary_key=True)
autofield.set_attributes_from_name("testid")
schema_editor.add_field(model=model, field=autofield)


def _create_db_table_for_model(model, schema_editor):
"""
Use the schema editor API to create the db table for the given (unmanaged) model.
"""
schema_editor.create_model(model)
if model._meta.model_name in _MODELS_WITH_FAKE_COMPOSITE_PK:
_replace_primary_key_field_with_autofield(model, schema_editor)
from django.db import connections


def create_db_tables_for_unmanaged_models(schema_editor):
Expand All @@ -65,7 +10,7 @@ def create_db_tables_for_unmanaged_models(schema_editor):
for model in appconfig.get_models():
if model._meta.managed:
continue
_create_db_table_for_model(model, schema_editor)
schema_editor.create_model(model)


def destroy_db_tables_for_unmanaged_models(schema_editor):
Expand Down
Loading