Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#31021 closed Bug (fixed)

0011_update_proxy_permissions crashes in multi database environment.

Reported by: haudoing Owned by: felixxm
Component: contrib.auth Version: 2.2
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by haudoing)

The tutorial said that we can omit to set the default database if default doesn't makes sense
https://docs.djangoproject.com/en/2.2/topics/db/multi-db/#defining-your-databases

But the following migration script doesn't work after configuration with empty default database
https://github.com/django/django/blob/stable/2.2.x/django/contrib/auth/migrations/0011_update_proxy_permissions.py

on line 42, it use

            with transaction.atomic():
                Permission.objects.filter(
                    permissions_query,
                    content_type=old_content_type,
                ).update(content_type=new_content_type)

This will brake the migration if default database doesn't set

Tracebacks

    raise ImproperlyConfigured("settings.DATABASES is improperly configured. "
django.core.exceptions.ImproperlyConfigured: settings.DATABASES is improperly configured. Please supply the ENGINE value. Check settings documentation for more details.

Attachments (1)

dj_2_2.zip (24.1 KB) - added by haudoing 3 months ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 3 months ago by felixxm

Resolution: worksforme
Status: newclosed
Summary: migration doesn't work on multi database environmentmigration doesn't work on multi database environment.

This migration doesn't brake anything for me. I checked with an example from docs and it doesn't require a default DB.

comment:2 Changed 3 months ago by haudoing

Sorry for my poor English. I doesn't express it well. What I mean to say is not migration script brake the multi db setting. I want to say after configure multi database setting without default, the migration script won't work. It throw the following exception while migrate

    raise ImproperlyConfigured("settings.DATABASES is improperly configured. "
django.core.exceptions.ImproperlyConfigured: settings.DATABASES is improperly configured. Please supply the ENGINE value. Check settings documentation for more details.

comment:3 Changed 3 months ago by haudoing

Resolution: worksforme
Status: closednew

comment:4 Changed 3 months ago by haudoing

Description: modified (diff)

comment:5 Changed 3 months ago by haudoing

Description: modified (diff)

comment:6 Changed 3 months ago by felixxm

Resolution: needsinfo
Status: newclosed

I want to say after configure multi database setting without default, the migration script won't work. It throw the following exception while migrate...

Just like I said, it works for me with an example from docs, that doesn't require a default DB. Please don't reopen this ticket without providing a small, sample project which we can use to reproduce this issue. You can also try to one of support channels.

comment:7 Changed 3 months ago by haudoing

Hello Felixxm,

Thank you for your response. I found the actual reason cause this issue.
I do start a clean Django project with the example you provide, it really works fine.
But when I create a proxy model then migration stop at that script again.
The script of the proxy model I made is as following

```class EditorUser(User):

    class Meta:
        proxy = True

    def get_author_name(self):
        '''
        Returns:
            a name leading by last_name
        '''
        return '{}{}'.format(self.last_name, self.first_name)```

comment:8 Changed 3 months ago by felixxm

I cannot reproduce this issue even with a proxy model :| . Can you attach a sample project?

Changed 3 months ago by haudoing

Attachment: dj_2_2.zip added

comment:9 Changed 3 months ago by haudoing

Thank you Felixxm.

I've attach a zip file with a very simple Django project. That should reproduce this issue.

comment:10 Changed 3 months ago by haudoing

Sorry Felixxm,

The example is fine for brand new django 2.2.
It only appear when upgrade from 2.1 to 2.2

comment:11 Changed 3 months ago by felixxm

I've tried to reproduce this issue with your project by following these steps:

  • pip install Django~=2.1.0
  • python manage.py migrate --database=auth_db
  • python manage.py migrate --database=others
  • pip install Django~=2.2.0
  • python manage.py migrate --database=auth_db
  • python manage.py migrate --database=others

and ... all works for me.

comment:12 Changed 3 months ago by haudoing

Please add a steps while working on Django 2.1 before migrate.

python manage.py makemigrations

That should reproduce this exception.
Though I'm not sure is it qualified to be a bug now.
After all A.B version upgrade doesn't 100% guarantee backward compatible.
But if it doesn't, it should be included in the release note, which it doesn't.
https://docs.djangoproject.com/en/2.2/releases/2.2/#permissions-for-proxy-models
Gotta thank you again felixxm!

comment:13 Changed 3 months ago by felixxm

Resolution: needsinfo
Severity: NormalRelease blocker
Status: closednew
Summary: migration doesn't work on multi database environment.0011_update_proxy_permissions crashes in multi database environment.
Triage Stage: UnreviewedAccepted

Many thanks! I was able to reproduce this issue, fix is quite simple:

diff --git a/django/contrib/auth/migrations/0011_update_proxy_permissions.py b/django/contrib/auth/migrations/0011_update_proxy_permissions.py
index c3f617f438..62e0a91737 100644
--- a/django/contrib/auth/migrations/0011_update_proxy_permissions.py
+++ b/django/contrib/auth/migrations/0011_update_proxy_permissions.py
@@ -39,7 +39,7 @@ def update_proxy_model_permissions(apps, schema_editor, reverse=False):
         old_content_type = proxy_content_type if reverse else concrete_content_type
         new_content_type = concrete_content_type if reverse else proxy_content_type
         try:
-            with transaction.atomic():
+            with transaction.atomic(using=schema_editor.connection.alias):
                 Permission.objects.filter(
                     permissions_query,
                     content_type=old_content_type,

comment:15 Changed 3 months ago by felixxm

Owner: changed from nobody to felixxm
Status: newassigned

comment:16 Changed 3 months ago by felixxm

Has patch: set

comment:17 Changed 3 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In e8fcdaad:

Fixed #31021 -- Fixed proxy model permissions data migration crash with a multiple databases setup.

Regression in 98296f86b340c8c9c968375d59f1d3a3479e60c2.

comment:18 Changed 3 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In ca9144a4:

[3.0.x] Fixed #31021 -- Fixed proxy model permissions data migration crash with a multiple databases setup.

Regression in 98296f86b340c8c9c968375d59f1d3a3479e60c2.

Backport of e8fcdaad5c428878d0a5d6ba820d957013f75595 from master

comment:19 Changed 3 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 9a17ae50:

[2.2.x] Fixed #31021 -- Fixed proxy model permissions data migration crash with a multiple databases setup.

Regression in 98296f86b340c8c9c968375d59f1d3a3479e60c2.

Backport of e8fcdaad5c428878d0a5d6ba820d957013f75595 from master

Note: See TracTickets for help on using tickets.
Back to Top