Page MenuHomeSoftware Heritage

django: Use valid identifiers for application labels
ClosedPublic

Authored by anlambert on Fri, Jan 8, 6:24 PM.

Details

Summary

While I was locally playing to change swh-web database backend from SQLite to PostgreSQL,
I stumbled across that error while trying to load a JSON dump of the webapp database into
the new Postgres one.

17:25 $ django-admin loaddata datadump.json --settings=swh.web.settings.development
[08/Jan/2021 16:26:07] [DEBUG] urllib3.util.retry.from_int:332 - Converted retries value: 1 -> Retry(total=1, connect=None, read=None, redirect=None, status=None)
[08/Jan/2021 16:26:07] [DEBUG] urllib3.util.retry.from_int:332 - Converted retries value: 1 -> Retry(total=1, connect=None, read=None, redirect=None, status=None)
Traceback (most recent call last):
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/core/serializers/json.py", line 69, in Deserializer
    yield from PythonDeserializer(objects, **options)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/core/serializers/python.py", line 92, in Deserializer
    Model = _get_model(d["model"])
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/core/serializers/python.py", line 154, in _get_model
    return apps.get_model(model_identifier)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/apps/registry.py", line 206, in get_model
    app_label, model_name = app_label.split(".")
ValueError: too many values to unpack (expected 2)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/anlambert/.virtualenvs/swh/bin/django-admin", line 8, in <module>
    sys.exit(execute_from_command_line())
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/__init__.py", line 381, in execute_from_command_line
    utility.execute()
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/__init__.py", line 375, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/base.py", line 323, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/base.py", line 364, in execute
    output = self.handle(*args, **options)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/commands/loaddata.py", line 72, in handle
    self.loaddata(fixture_labels)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/commands/loaddata.py", line 114, in loaddata
    self.load_label(fixture_label)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/commands/loaddata.py", line 172, in load_label
    for obj in objects:
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/core/serializers/json.py", line 73, in Deserializer
    raise DeserializationError() from exc
django.core.serializers.base.DeserializationError: Problem installing fixture '/home/anlambert/swh/swh-environment/swh-web/datadump.json':

This is due to the fact that swh-web declares the following labels for
some custom django applications: swh.web.common, swh.web.auth.

But Django application label must be a valid Python identifier and thus must
not contain dot characters.

As a result, having invalid identifiers will make database data importing from JSON
fails.

This was not clearly indicated in Django documentation and application label validity
check has been added a couple of weeks ago in upstream Django source code.
https://code.djangoproject.com/ticket/32285

That diff renames the invalid django application labels and ensure migrations can
still be applied.

In order to apply the renaming changes to the swh-web instance in production,
the migration command will have to be the following:

django-admin migrate --settings=swh.web.settings.production --fake

This will simulate the migrations but without any effects apart filling the django migrations
table with the new application labels.

Calling migrate without the fake option will raise errors otherwise as the changes
have already been applied to database, see below.

17:38 $ django-admin migrate --settings=swh.web.settings.development
[08/Jan/2021 16:38:12] [DEBUG] urllib3.util.retry.from_int:332 - Converted retries value: 1 -> Retry(total=1, connect=None, read=None, redirect=None, status=None)
[08/Jan/2021 16:38:12] [DEBUG] urllib3.util.retry.from_int:332 - Converted retries value: 1 -> Retry(total=1, connect=None, read=None, redirect=None, status=None)
Operations to perform:
  Apply all migrations: admin, auth, contenttypes, sessions, swh_web_auth, swh_web_common
Running migrations:
  Applying swh_web_auth.0001_initial...Traceback (most recent call last):
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/db/backends/utils.py", line 82, in _execute
    return self.cursor.execute(sql)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/django/db/backends/sqlite3/base.py", line 381, in execute
    return Database.Cursor.execute(self, query)
sqlite3.OperationalError: table "oidc_user_offline_tokens" already exists

This could be implemented the following way in debian/postinst:

#!/bin/bash

set -e

if [ -f /var/lib/swh/web.sqlite3 ]; then
    echo "Software Heritage production db detected; running migrations"
    if [ -d /usr/lib/python3/dist-packages/swh.web-0.0.279.egg-info/ ]
    then
        # use fake migrations after django app labels renaming in v0.0.279
        django-admin migrate --settings=swh.web.settings.production --fake
    else
        django-admin migrate --settings=swh.web.settings.production
    fi
fi

Related to T2945

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D4830 (id=17111)

Rebasing onto e605c3fa70...

Current branch diff-target is up to date.
Changes applied before test
commit 7b862d847ebc76adf9c20a9de45fc96065dd76a9
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Fri Jan 8 12:46:21 2021 +0100

    django: Use valid identifiers for app labels
    
    Django application label must be a valid Python identifiers and thus
    must not contain dot characters.
    
    Having invalid identifiers will make database data importing from JSON
    fails otherwise.
    
    Related django issue: https://code.djangoproject.com/ticket/32285
    
    Related to T2945

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/550/ for more details.

This revision is now accepted and ready to land.Fri, Jan 8, 6:49 PM

Build is green

Patch application report for D4830 (id=17474)

Rebasing onto e702c28aee...

Current branch diff-target is up to date.
Changes applied before test
commit b71d14deebe884b831542276242e55dd6fe3ed15
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Fri Jan 8 12:46:21 2021 +0100

    django: Use valid identifiers for app labels
    
    Django application label must be a valid Python identifiers and thus
    must not contain dot characters.
    
    Having invalid identifiers will make database data importing from JSON
    fails otherwise.
    
    Related django issue: https://code.djangoproject.com/ticket/32285
    
    Related to T2945

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/556/ for more details.