Make deconstructible classes forwards compatible
Markus Holtermann
As of now (Django 1.7.x and master) the migration writer serializes classes like the CreateModel operation with kwargs and not args. Which is the right way to do. However, as soon as Django changes some of these operations' constructor signatures (as I plan to do in https://code.djangoproject.com/ticket/23822) newly created migration files become backwards incompatible for 3rd party apps.
As a concrete example: CreateModel will get a "managers" argument in 1.8 (if my pull request will be accepted). Hence all migration files created with Django 1.8 will contain a managers argument as well. Third party apps that want to provide compatibility for 1.7 and 1.8 will have a problem as managers is an unknown kwarg to CreateModel in 1.7.
I therefore propose to add **kwargs to all operations and would like that to be backported to 1.7 although its neither a bug nor security fix.
I haven't looked into other classes that are serialized by the migration writer if they would profit from the same patch, but from my perspective model fields don't need that change as their signature didn't change over "a decade".
Another possible problem I want to address but that I haven't looked into yet, is the serialization of args in general. I think having **kwargs for every deconstructible class and always serialize with kwargs and not having any args in the migration classes will prevent problems in the future, too.
Markus
Carl Meyer
On 11/21/2014 05:35 AM, Markus Holtermann wrote:
> As of now (Django 1.7.x and master) the migration writer serializes
> classes like the CreateModel operation with kwargs and not args.
> Which is the right way to do. However, as soon as Django changes some
> of these operations' constructor signatures (as I plan to do in
> https://code.djangoproject.com/ticket/23822) newly created migration
> files become backwards incompatible for 3rd party apps.
>
> As a concrete example: CreateModel will get a "managers" argument in
> 1.8 (if my pull request will be accepted). Hence all migration files
> created with Django 1.8 will contain a managers argument as well.
> Third party apps that want to provide compatibility for 1.7 and 1.8
> will have a problem as managers is an unknown kwarg to CreateModel in
> 1.7.
>
> I therefore propose to add **kwargs to all operations and would like
> that to be backported to 1.7 although its neither a bug nor security
> fix.
> I haven't looked into other classes that are serialized by the
> migration writer if they would profit from the same patch, but from
> my perspective model fields don't need that change as their signature
> didn't change over "a decade".
>
> Another possible problem I want to address but that I haven't looked
> into yet, is the serialization of args in general. I think having
> **kwargs for every deconstructible class and always serialize with
> kwargs and not having any args in the migration classes will prevent
> problems in the future, too.
to the implementor of `deconstruct()` for any particular class to decide
which things to record as args and which as kwargs, and to take into
account the backwards-compatibility considerations. It's not possible in
the general case to "always serialize with kwargs", since some objects
might accept some things only via `*args`.
If you just mean looking through the implementations of `deconstruct()`
in Django itself and using more kwargs and fewer args there, I think
that's a good idea.
Carl
Markus Holtermann
On Friday, November 21, 2014 7:57:47 PM UTC+1, Carl Meyer wrote:
Hi Markus,
On 11/21/2014 05:35 AM, Markus Holtermann wrote:
> As of now (Django 1.7.x and master) the migration writer serializes
> classes like the CreateModel operation with kwargs and not args.
> Which is the right way to do. However, as soon as Django changes some
> of these operations' constructor signatures (as I plan to do in
> https://code.djangoproject.com/ticket/23822) newly created migration
> files become backwards incompatible for 3rd party apps.
>
> As a concrete example: CreateModel will get a "managers" argument in
> 1.8 (if my pull request will be accepted). Hence all migration files
> created with Django 1.8 will contain a managers argument as well.
> Third party apps that want to provide compatibility for 1.7 and 1.8
> will have a problem as managers is an unknown kwarg to CreateModel in
> 1.7.
>
> I therefore propose to add **kwargs to all operations and would like
> that to be backported to 1.7 although its neither a bug nor security
> fix.
This makes sense to me.
> I haven't looked into other classes that are serialized by the
> migration writer if they would profit from the same patch, but from
> my perspective model fields don't need that change as their signature
> didn't change over "a decade".
>
> Another possible problem I want to address but that I haven't looked
> into yet, is the serialization of args in general. I think having
> **kwargs for every deconstructible class and always serialize with
> kwargs and not having any args in the migration classes will prevent
> problems in the future, too.
I'm not sure how you're proposing to address this? It seems like it's up
to the implementor of `deconstruct()` for any particular class to decide
which things to record as args and which as kwargs, and to take into
account the backwards-compatibility considerations. It's not possible in
the general case to "always serialize with kwargs", since some objects
might accept some things only via `*args`.
If you just mean looking through the implementations of `deconstruct()`
in Django itself and using more kwargs and fewer args there, I think
that's a good idea.