cortex-lab / cortex-lab/alyx

Make db fields independent of IO

Open
#1,045 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
52
Forks
12
Avg merge
1d 11h
Merged PRs (30d)
4

Description

Model.__init__ calls field.get_default() for every field you don't pass:

val = field.get_default()      # django/db/models/base.py, three call sites

So Subject(nickname='x') fires three queries just to construct an object in memory before any save, and three of these defaults use get_or_create:

def default_dataset_type():
    return DatasetType.objects.get_or_create(name='unknown')[0].pk

which means constructing an in-memory Dataset can write to the database. In a DRF create() path, in a transaction that later rolls back, or — during a release — against the wrong database entirely. (Loading from the DB is safe: from_dbuses positional args and skips defaults.)

Triage
Default Queries? What to do
default_species get_or_create, pk already hardcoded return the literal pk; seed the row by fixture
default_dataset_type, default_data_format get_or_create by name pin a UUID for each 'unknown' row, seed by fixture, return the literal
default_source filter().first(), returns an instance pin similarly — note it returns an instance where the others return pks, which is its own inconsistency
default_responsible "first stock manager" genuinely dynamic — cannot be a constant, so it has to move off the field

For the first four, the value is fixed; the query only exists to guarantee the row exists, and that is a data concern belonging in a fixture or data migration, not in a default. Alyx already ships fixtures for exactly this kind of seed data. In Django 5, db_default is the idiomatic home for the constant cases — it pushes the default into the column so the database supplies it and Python never evaluates anything. It's accepted on related fields.

For default_responsible, the first stock manager can't be a column default, so it moves to where the object is actually created — Subject.save(), or the admin, where BaseAdmin.get_changeform_initial_data already exists for precisely this sort of prefill.

Changing a default alters the field's deconstruction, so each one needs an AlterField migration or makemigrations --check will fail.

Guards

In the Open Alyx 01b_prune script we use:

def _enforce_default_read_only(sender, connection, **kwargs):
    if connection.alias == 'default':
        cursor.execute('SET default_transaction_read_only = ON;')

Extending that to 01a's migrate step would protect against future writes to production during migrate --database public.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in django/db/models/base.py at the three field.get_default() call sites, then locate default_species, default_dataset_type, default_data_format, default_source, and default_responsible. Review Subject.save(), BaseAdmin.get_changeform_initial_data, the Open Alyx 01a and 01b scripts, and existing fixtures; done means constants no longer query during construction, dynamic responsibility is handled at creation, seed data is protected, and makemigrations --check passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
django, python
Domain
backend, database
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.