[Django] #25068: Metaclass conflict when doing createmigrations in ModelState.render
Django
-------------------------+-------------------------------------------------
Reporter: kosz85 | Owner: nobody
Type: | Status: new
Uncategorized |
Component: | Version: 1.8
Migrations |
Severity: Release | Keywords: metaclass conflict createmigrations
blocker |
Triage Stage: | Has patch: 0
Unreviewed |
Easy pickings: 0 | UI/UX: 0
-------------------------+-------------------------------------------------
I'm migrating my project from Django 1.6.x to 1.8.2, I found this bug and
fix for it.
In my project I'm using few ModelMixins with custom {{{__metadata__}}},
problem is that render is trying do a class using {{{type}}} but this code
forgot about python class metadata multi inheritance.
Traceback looks like this, for each model with more complex metadata:
{{{
$ /manage.py makemigrations
(...)
Traceback (most recent call last):
File "/home/vagrant/***/manage.py", line 10, in <module>
execute_from_command_line(sys.argv)
File "/home/vagrant/env/local/lib/python2.7/site-
packages/django/core/management/__init__.py", line 338, in
execute_from_command_line
utility.execute()
File "/home/vagrant/env/local/lib/python2.7/site-
packages/django/core/management/__init__.py", line 330, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/vagrant/env/local/lib/python2.7/site-
packages/django/core/management/base.py", line 390, in run_from_argv
self.execute(*args, **cmd_options)
File "/home/vagrant/env/local/lib/python2.7/site-
packages/django/core/management/base.py", line 441, in execute
output = self.handle(*args, **options)
File "/home/vagrant/env/local/lib/python2.7/site-
packages/django/core/management/commands/makemigrations.py", line 125, in
handle
migration_name=self.migration_name,
File "/home/vagrant/env/local/lib/python2.7/site-
packages/django/db/migrations/autodetector.py", line 43, in changes
changes = self._detect_changes(convert_apps, graph)
File "/home/vagrant/env/local/lib/python2.7/site-
packages/django/db/migrations/autodetector.py", line 110, in
_detect_changes
self.old_apps = self.from_state.concrete_apps
File "/home/vagrant/env/local/lib/python2.7/site-
packages/django/db/migrations/state.py", line 170, in concrete_apps
self.apps = StateApps(self.real_apps, self.models,
ignore_swappable=True)
File "/home/vagrant/env/local/lib/python2.7/site-
packages/django/db/migrations/state.py", line 232, in __init__
self.render_multiple(list(models.values()) + self.real_models)
File "/home/vagrant/env/local/lib/python2.7/site-
packages/django/db/migrations/state.py", line 262, in render_multiple
model.render(self)
File "/home/vagrant/env/local/lib/python2.7/site-
packages/django/db/migrations/state.py", line 548, in render
body,
TypeError: metaclass conflict: the metaclass of a derived class must be a
(non-strict) subclass of the metaclasses of all its bases
}}}
Fix is simple. I used classmaker from here
http://code.activestate.com/recipes/204197-solving-the-metaclass-conflict/
And replaced line 543 of db/migrations/state.py from this:
{{{
# Then, make a Model object (apps.register_model is called in
__new__)
return type(
str(self.name),
bases,
body,
)
}}}
to this:
{{{
# Then, make a Model object (apps.register_model is called in
__new__)
from lib.utils.meta_maker import classmaker
return classmaker()(str(self.name), bases, body)
}}}
My lib.utils.meta_maker is code from link I provided, with little changes.
This way it works without a problems as metaclases are handled properly.
Classmaker in most cases will return type, but for my more complex models
will do the magic.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: metaclass conflict | Triage Stage:
createmigrations | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* type: Uncategorized => Bug
* needs_tests: => 0
* needs_docs: => 0
Comment:
Thanks for the report. We'll need a regression test so we can reproduce
the issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:1>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: metaclass conflict | Triage Stage:
createmigrations | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by kosz85):
* Attachment "metaconflicts.zip" added.
Metaconflicts isolated example
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: metaclass conflict | Triage Stage:
createmigrations | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by kosz85):
I attached isolated case from my project. There is also version of
metaclassmaker that I use, it's same as in the link. I know that playing
with multiple inheritance and metaclasses is not so simple, but with
metaclassmaker it's also not a problem, and I get quite nice features at
model level like all subclasses (and subsubsubsub classes ) aware class
registry, or some class aware lists, and other magic tools.
With this one you can create all things like Notifications, LogTypes,
IndexableMixins, SyncMixins, ..., with automatic population of all ever
created classes of objects in nice and neat way.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:2>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: metaclass conflict | Triage Stage:
createmigrations | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by timgraham):
It looks like there's no changes needed to make this work on Python 3. I'm
in favor of "won't fix" given the complexity of bundling `metaclassmaker`
in Django and the timetable for dropping Python 2 support. Thoughts?
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:3>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: metaclass conflict | Triage Stage:
createmigrations | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by kosz85):
The same problem will occur at Python 3 probably, as metaclasses mechanics
didn't change too much. If you want I can work on version for Python 3
also, I'm thinking about migrating my code anyway.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:4>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: metaclass conflict | Triage Stage:
createmigrations | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by kosz85):
* Attachment "metaconflicts.zip" added.
Metaconflicts isolated example - with six (tested on 2.7 & 3.4)
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: metaclass conflict | Triage Stage:
createmigrations | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by kosz85):
I updated example, I used six to make it more generic. The problem exists
in both python 2.7 and 3.4, I added new method
{{{six_with_metaclassmaker}}} it's similar to {{{six.with_metaclass}}} but
more specific for this case, as {{{six.with_metaclass}}} and
{{{six.add_metaclass}}} aren't made for such cases:
- The first (correct) approach is assuming that meta is already a class,
and not function like in 2.7, but metaclassmaker is just a factory of
proper type and not type class itself.
- Second approach (incorrect) can't deal with class that rise error itself
because of metaclass inheritance we want to deal with.
- So third approach is copy of first approach but using {{{type}}} as base
for proxy class for {{{metaclassmaker}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:5>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: metaclass conflict | Triage Stage:
createmigrations | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by timgraham):
Could you put together a patch so we can more easily see what's involved
in fixing it?
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:6>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: metaclass conflict | Triage Stage:
createmigrations | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by kosz85):
Hah, I wanted to avoid it, as I have some nasty deadlines, but ok I will
try to do it this or next week, be patient.
Just help me decide where I should put the code with metaclassmaker? As
db/migrations/metaclassmaker.py or you have some particular place for such
internal tools? I can also try do it a part of render method or state
module, though I think encapsulation is better way of doing it.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:7>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by timgraham):
* severity: Release blocker => Normal
* stage: Unreviewed => Accepted
Comment:
I guess somewhere in `django.utils` would probably make sense. I'll
tentatively accept the ticket for now.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:8>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by kosz85):
Added pull request
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:9>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by kosz85):
https://github.com/django/django/pull/4968
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:10>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Needs tests: 0 | Patch needs improvement: 0
Changes (by timgraham):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:11>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by MarkusH):
I'm not really happy with bundling `metaclassmaker` either. Furthermore, I
think there are a few problems with your current implementation:
1. What happens if a model's metaclass changes from `MyModelBase` to
Django's `ModelBase` or vice versa?
1. I don't see an explicit tracking of the class in generated migration
files, do I?
1. As the metaclass has to be tracked inside migrations (only which
metaclass, what the metaclass does) but is only referenced from the
regular code base, can we find a solution like for the model managers?
1. Keep in mind, that any fields a metaclass adds to a model are
automatically serialized (see e.g. abstract models).
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:12>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:13>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by kosz85):
Yeah, It's quite hard topic, but your current solution just doesn't work
with code that work on South, and you have to take into account, that
there may be more then 1 metaclass. Let's discuss and think on some
solution but first one thing:
'''All your points are occurring also in current version. My fix is only
dealing with multiple inheritance of metaclasses.''' Currently you also
can have custom metaclass and you can change it without any note at
migrations, the only thing is that current migrations support only 1 such
metaclass now, and break on multiple inheritance.
1. I don't understand your question :) It would be changed, and derived
class would be changed. It's rather unlikely that you will be changing
`ModelBase` itself, as it's more like you want to just add some metaclass
functionality. The new metaclass will have both functionalities from
`ModelBase` and your custom metaclass. If I understand correctly what you
would like to do, so to change the custom `MyModelBase` with `ModelBase`
it probably would work in most case, but you will need to modify base
classes to be able to create class object. In my opinion this solution
would be even scarier.
2. No I didn't do anything like that. But do we really need it? Currently
`BaseModel` is also not listed, and if I will add normal metaclass with
single not multiple inheritance it will work the same way as it's now
without any notes that there is different metaclass. But of course it's
probably possible to list metaclasses, but I don't really think it's
needed. The change in fileds will be noticed, options are still part of
`BaseModel`, custom metacalss doesn't change it. In most cases custom
metaclasses are for some tricks, like global registry, singletons, class
definitions validators, and they are not used for dealing with models
itself. That in my case, maybe I'm wrong, but I don't see much more danger
here comparing to current solution.
3. It's possible to list metaclasses, the code that is constructing
metaclass is doing it in fact it's just `map(type, bases)` and you can go
with it like you will do with normal bases, as metaclass is also a class,
but with base of `type`. So it's possible to reuse same code for
construction bases list. Thought managers are quite different being.
4. Yes, and I don't see any problem with it. Thought as I said before, I
don't think it's this kind of metaclass :) Normally I would use 2
different mixins for custom metaclass and abstract Model with fields, as
it would be cleaner and easier to maintain. Also that way I can reuse
metaclass in many models. But of course you can also try to inherit your
metaclass from ModelBase and made it custom, but you can do it even now
and you have to deal with same problems you listed, but without error I
reported. My fix solves only multiple inheritance problem, with witch
`ModelState` couldn't deal with.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:14>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by kosz85):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:15>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Marking as "Patch needs improvement" since the current pull request
[https://github.com/django/django/pull/4968#issuecomment-133674517 results
in a performance regression].
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:16>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by kosz85):
I upgraded it, now it shouldn't be much slower for normal cases, but will
be a bit slower for Models that use metaclasses. Please check again, I
wait for this to be merged, I don't like idea to support own django fork
:)
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:17>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:18>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
createmigrations | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by MarkusH):
* stage: Accepted => Ready for checkin
Comment:
Patch looks good to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:19>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
Patch still scares me and could be motivated with documentation about the
use cases I think.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:20>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by kosz85):
That's not smth to be scared of. I can explain what is happening there:
Each class has metaclass, which default is `type`. It's class of class.
You can change it and made your own type declaring metaclass. What new
migrations did, is bad assumption that class would be created only using
`type`, and that there would be only simple conflicts.
First take a note that `Model` metaclass is already set to `BaseModel`.
Assume such situation:
class cA with metaclass mA
class cB with metaclass mB
class cC(cA, cB) -> metaclass mC
Python has no problem with standard metaclasses.
When mA and mB eguals default `type` then python knows that mC is also
`type`.
Also when mA or mB equals default `type` and the other not (for example
`BaseModel`) then he can easily solve conflict because every metaclass is
derived from `type` so Python is using the other one so mC would be
`BaseModel`.
But when mA is custom and mB is custom, he don't know what to do, and
there are even worse scenarios, there may be 3 or more bases to inherit
from. Then standard response is to manually build mC that inherit from mA
and mB (like in normal class inheritance).
So what is doing this patch is reacting to that `TypeError`, and
constructing (it's normal factory) such custom metaclass from metaclasses
that are used in bases of this class. That new metaclass is used then to
create object, instead of standard type. So no magic ;) The only magic is
factory and populating metaclasses from bases.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:21>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by MikeAmy):
Also encountered the same issues.
I've created a lighter pull request that fixes it for me and makes the
proposed test pass.
https://github.com/django/django/pull/5907
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:22>
Django
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:23>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by kosz85):
Thats the same what metaclassmaker was doing ;) you just did it inline
without naming function, great that it's fixed. Thx for your effort :)
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by loic):
I don't think this is the right approach.
The current status of inheritance in migrations:
- Inheritance is only partially supported (abstract models are ignored) in
`CreateModel()` with the `bases` kwarg.
- Inheritance changes aren't tracked over time.
- Metaclasses aren't supported.
With the exception of abstract models (ignoring them was a deliberate
design decision by Andrew), we could cleanly fix all the other
shortcomings by:
- Adding a `metaclass=ModelBase` kwarg to `CreateModel()`.
- Adding a `AlterModelClass(bases, metaclass)` operation.
Doing so would bring us closer to standard Python inheritance, support
changes over time, and replace the proposed runtime magic with a
declarative syntax.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:25>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: loic (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:26>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:27>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Yingtong Li):
It looks like it's been a while since there was any movement in this
ticket, but I've just encountered this issue myself (Django 1.10.4).
kosz85's patches were not sufficient for my use case, as my metaclass
influenced the database structure of the Model, and so needed to be used
rather than merely emulated.
I have put together some quick changes along the lines of Loic Bistuer's
suggestions, which are sufficient for my needs:
https://github.com/django/django/compare/master...RunasSudo:migrations-
with-metaclasses?expand=1
However, more work would be required to implement the rest of the
functionality and fully complete the integration of the changes with the
rest of Django, and I do not have the confidence in my own ability to do
so. Hopefully these changes may form the foundation for further changes if
anyone plans further work on this ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:28>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by kosz85):
It would be nice to have it integrated in django. I'm using custom builds
for latest LTS adding this bugfix.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:29>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ethan J. Howell):
Building off of @Yingtong Li's PR I submitted a new one that should solve
everything: https://github.com/django/django/pull/13384.
Although it would be nice to see if there's a short-term solution anyone
else has come up with.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:30>
Django
-------------------------------------+-------------------------------------
| Howell
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Markus Holtermann (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:33>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Ethan J.
| Howell
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:34>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Ethan J.
| Howell
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
createmigrations | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:35>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Ethan J.
| Howell
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:36>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Ethan J.
| Howell
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:38>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Ethan J.
| Howell
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:39>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Ethan J.
| Howell
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
The current solution doesn't track metaclass changes, see
[https://github.com/django/django/pull/13384#pullrequestreview-541793879
comment].
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:40>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Ethan J.
| Howell
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
This is probably worth another look. Even though changing metaclass bases
don't generate migrations, that seems like another flavor of #23521, no?
(That ticket seems to be a catch-all for "can't change my parent
classes").
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:41>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Ethan J.
| Howell
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):
* needs_tests: 1 => 0
Comment:
Of course, feel free to set the flags back if this really shouldn't move
forward without tracking metaclass changes.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:42>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Ethan J.
| Howell
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
IMO, we shouldn't move forward without tracking metaclass changes, see
[https://code.djangoproject.com/ticket/25068#comment:40 comment].
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:43>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Ethan J.
| Howell
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
Yeah, that makes sense safety-wise. I guess it's different enough from
#23521 in that there's more of an expectation here that metaclasses
can/will change.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:44>
Django
-------------------------------------+-------------------------------------
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Ethan J. Howell => Tom L.
--
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:45>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Tom L.
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
-------------------------------------+-------------------------------------
Changes (by Tom L.):
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:46>
Django
-------------------------------------+-------------------------------------
Reporter: kosz85 | Owner: Tom L.
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: metaclass conflict | Triage Stage: Accepted
createmigrations |
Has patch: 1 | Needs documentation: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Ticket URL: <https://code.djangoproject.com/ticket/25068#comment:47>