[Django] #26099: Migrations name conflict
Django
-------------------------------+--------------------
Reporter: LucidComplex | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
Django's automatic migration creation may suffer from a name conflict
without any warning.
Creating a migration for a model having a relation with
"django.contrib.auth.models.User" will cause Django to automatically
import "django.conf.settings" in migrations.
This is all fine and good, unless you have an app unfortunately named
"settings". Django will not fire a warning about this name conflict.
--
Ticket URL: <https://code.djangoproject.com/ticket/26099>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------+--------------------------------------
Reporter: LucidComplex | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by LucidComplex):
* needs_better_patch: => 0
* type: Uncategorized => Bug
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:1>
Django
-------------------------------+--------------------------------------
Type: Bug | Status: new
Component: Uncategorized | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by LucidComplex):
* version: 1.9 => 1.8
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:2>
Django
--------------------------------------+------------------------------------
Reporter: LucidComplex | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* type: Bug => Cleanup/optimization
* component: Uncategorized => Migrations
* stage: Unreviewed => Accepted
Comment:
It might be possible to automatically add some "Import x as y" shims as
necessary.
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:3>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by koterpillar):
* cc: a@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:4>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Anton Samarchyan):
So we want to have something like this if the app name is settings?
{{{
from __future__ import unicode_literals
from django.conf import settings as django_settings
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
initial = True
dependencies = [
migrations.swappable_dependency(django_settings.AUTH_USER_MODEL),
]
operations = [
migrations.CreateModel(
name='Test',
fields=[
('id', models.AutoField(auto_created=True,
primary_key=True, serialize=False, verbose_name='ID')),
('user', models.ForeignKey(blank=True, null=True,
on_delete=django.db.models.deletion.SET_NULL,
to=django_settings.AUTH_USER_MODEL)),
],
),
]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:5>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Alexey Kotlyarov):
This doesn't change anything _by itself_, as an app can still be named
`django_settings`.
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:6>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Anton Samarchyan):
* cc: desecho@… (added)
Comment:
What if first it's going to check if there is an app called settings, then
if there is it's going to choose the name django_settings and check if the
app exists again (with the name django_settings), if it does, it's going
to name it django_settings_settings for example and check again, etc. Does
it sound good?
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:7>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
There could be more import conflicts than just one for the name "settings"
so the approach should be more general. I'm not sure whether or not
solving this is feasible and/or practical.
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:8>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Alexey Kotlyarov):
The needed parts of Django can be imported without `from`, so they stay
inside `django` namespace:
{{{
import django.conf.settings
import django.db.migrations
import django.db.models
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:9>
Django
Reporter: LucidComplex | Owner: Alexey
Type: | Kotlyarov
Cleanup/optimization | Status: assigned
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Alexey Kotlyarov):
* status: new => assigned
* owner: nobody => Alexey Kotlyarov
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: LucidComplex | Owner: Alexey
Type: | Kotlyarov
Cleanup/optimization | Status: assigned
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Alexey Kotlyarov):
While making changes, I found that the migration tests don't check the
imports inside the migration files properly, and it was possible for them
to pass even though the resulting migrations wouldn't have run on their
own:
https://github.com/django/django/commit/7bd4c3e75f3d9f9bbd43c2d4a39b786f2d75137d
I have a commit that fixes the imports, but I need to figure out how to
make a test for the bad application name.
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: LucidComplex | Owner: Alexey
Type: | Kotlyarov
Cleanup/optimization | Status: assigned
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Alexey Kotlyarov):
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/7810
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: LucidComplex | Owner: Alexey
Type: | Kotlyarov
Cleanup/optimization | Status: assigned
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
Comment:
I'm not sure if that's a developer friendly change considering that
migrations can be authored and edited by hand. Defaulting to absolute
paths everywhere makes migration files more verbose and perhaps less
readable. I'd want a consensus on the DevelopersMailingList on this change
before proceeding.
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: LucidComplex | Owner: Alexey
Type: | Kotlyarov
Component: Migrations | Version: 1.8
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Mariusz Felisiak):
* status: assigned => closed
* resolution: => needsinfo
* stage: Accepted => Unreviewed
Comment:
> I'd want a consensus on the DevelopersMailingList on this change before
proceeding.
Marking as `needsinfo` as we should first reach a consensus about the
approach.
--
Ticket URL: <https://code.djangoproject.com/ticket/26099#comment:14>