Dynamic fields (via contrib_to_class) no longer work.

[Stephen McDonald]

Hi Andrew, all.

I'm the developer of some moderately popular Django projects, Mezzanine [1] and Cartridge [2], which are a CMS/Ecommerce combination that have had a good community built up around them over the past few years of development. We've been using south for the majority of this time and for the most part it's just been this magic piece of infrastructure that has just worked - so thanks for all your effort in helping us to ease the pain of migration for all our users.

In Mezzanine we have a set of reusable fields [3] for ratings, comments and keywords on models. What these fields have in common is that they each use the approach of contribute_to_class to add extra fields to the model they're attributed to, where some denormalized data gets stored - for example the rating field will store the rating counts and averages against the model each time a rating is added.

We've been using this approach for (at a guess) well over a year, and south has not had any problems with these dynamic fields in the past. At some point recently though we started to notice some issues. At first we'd get errors such as "multiple assignments to same column" when running migrations, which brought me to ticket #327 [4] and a handful of solutions to the problem, which I tried to implement [5] and seems to work for the most part.

I really have to plead ignorance at the inner-workings of south, so please forgive me if some of my questions seem incomplete. As I mentioned above, south has always been this black box behind the scenes that has just worked. As I'm sure you can relate to, at the best of times it's been a struggle to find enough time in life to keep up to date with the community around the software you're trying to manage and maintain, so my knowledge of the complexities of south remains small to none. 

My questions are:

- With the above change made to Mezzanine, we can no longer use the --auto option with the schemamigration command as Django's FieldDoesNotExist gets raised. So we have to create migrations by hand now if they don't fall under one of the other options (--add-field, etc). Does this seem right? How can we get --auto working again?

- We're now getting error reports from users creating new projects saying that our initial/early migrations in the project that were created a long time ago now cause errors with missing relations on these custom fields [6]. How can we prevent this from happening? Should we go back and patch migrations to use the new tweaks we've had to do? 

At a broader level I'm really looking for some general advice going forward:

- Should we as an open source project implemented as Django apps, even be providing migrations in the project? This has worked well for us over the years, but it seems like we could possibly avoid these types of problems by putting the onus on users to implement south in their own Mezzanine projects themselves. I really don't want to do this - with a project as broad as a CMS, we go a long way to bridging the gap between new users who are used to just uploading PHP files to a server, and who are now looking at a whole new realm of concepts with developing and maintaining Django projects. Providing these migrations has been one key part of bridging that gap.

- Does all of this point to a particular change in south? Is there a particular version of south we could pin to prior to this change that would avoid this issue? I know that doesn't sound very future proof, but from our perspective this has worked for a long time, and now it all seems to have come apart at the seams. 

- If all of this does point to a particular change in south, what are the chances of it being corrected? From the conversation in the original issue reported (#327) it seems like it's not considered a problem and won't be changing. Again I really don't know enough about the inner-workings of south to classify it as a bug or not.

- Finally, and I hate to ask, but in the long term, does this entire dynamic field approach simply conflict with how south works at its core? Are these two things never going to gel together cleanly as they once did? If not, does anyone know of any alternatives to south that might be better suited for our project?

Thanks for your time, and again for all of the effort that has gone into some obviously very complicated software.

[1] http://mezzanine.jupo.org/

[2] http://cartridge.jupo.org/

[3] https://github.com/stephenmcd/mezzanine/blob/master/mezzanine/generic/fields.py

[4] http://south.aeracode.org/ticket/327

[5] https://github.com/stephenmcd/mezzanine/commit/a302a68c19865bd7e992cab5d490b7c64aac95f2

[6] https://groups.google.com/group/mezzanine-users/browse_thread/thread/aff8342ed77960c7

Cheers,

Steve

--
Stephen McDonald
http://jupo.org

[Andrew Godwin]

On 04/07/12 22:20, Stephen McDonald wrote:
> Hi Andrew, all.

Hi Stephen.

> My questions are:
>
> - With the above change made to Mezzanine, we can no longer use the
> --auto option with the schemamigration command as Django's
> FieldDoesNotExist gets raised. So we have to create migrations by hand
> now if they don't fall under one of the other options (--add-field,
> etc). Does this seem right? How can we get --auto working again?

This is because you're doing things beyond what South can cope with - it
was never designed to handle fields that appear dynamically at runtime,
and so I'm afraid --auto just isn't going to be reliable. The only
solution on the South end would be a major refactor - this is already
underway, but nowhere near even alpha status yet.

> - We're now getting error reports from users creating new projects
> saying that our initial/early migrations in the project that were
> created a long time ago now cause errors with missing relations on these
> custom fields [6]. How can we prevent this from happening? Should we go
> back and patch migrations to use the new tweaks we've had to do?

You share this issue with Django-CMS - it's probably because you have
fields that don't confirm to the strict migration guarantee of "nothing
must ever vanish". Your only option here is to start afresh with a new
set of initial migrations and post upgrade instructions for users on how
to get to them (migrate --fake probably needs to be applied several
times). This isn't a very good solution, but the only one available
right now, alas.

> At a broader level I'm really looking for some general advice going forward:
>
> - Should we as an open source project implemented as Django apps, even
> be providing migrations in the project? This has worked well for us over
> the years, but it seems like we could possibly avoid these types of
> problems by putting the onus on users to implement south in their own
> Mezzanine projects themselves. I really don't want to do this - with a
> project as broad as a CMS, we go a long way to bridging the gap between
> new users who are used to just uploading PHP files to a server, and who
> are now looking at a whole new realm of concepts with developing and
> maintaining Django projects. Providing these migrations has been one key
> part of bridging that gap.

You should be able to, though I agree South is not perfect considering
your database wizardry. Making users provide migrations is wrong,
however - you should be able to release a new version with new
models/etc. and users should be able to upgrade to it easily using
supplied migrations.

>
> - Does all of this point to a particular change in south? Is there a
> particular version of south we could pin to prior to this change that
> would avoid this issue? I know that doesn't sound very future proof, but
> from our perspective this has worked for a long time, and now it all
> seems to have come apart at the seams.

Not really - it's just the continual march of bugfixes and tightening
constraints, along with an aging migration codebase, which brings its
own set of problems. This feeds into the next few questions.

>
> - If all of this does point to a particular change in south, what are
> the chances of it being corrected? From the conversation in the original
> issue reported (#327) it seems like it's not considered a problem and
> won't be changing. Again I really don't know enough about the
> inner-workings of south to classify it as a bug or not.

I'll combine my answer to this with the next question.

> - Finally, and I hate to ask, but in the long term, does this entire
> dynamic field approach simply conflict with how south works at its core?
> Are these two things never going to gel together cleanly as they once
> did? If not, does anyone know of any alternatives to south that might be
> better suited for our project?
>
> Thanks for your time, and again for all of the effort that has gone into
> some obviously very complicated software.

So, at some level, yes, your approach to fields does conflict with how
South works - schemas are very fixed, immovable things, and South is
designed to bash them into shape in certain ways based on the common
Django patterns. There's no migration framework I know of that will
handle your problems (at least, not one that will work across databases
nicely).

However, I'm currently in the middle of both extending Django itself and
rewriting South to incorporate the lessons and problems of the last
three years and try and considerably improve things. I can't promise
this will lead to better handling of contribute_to_class fields, but
I'll definitely keep it in mind - it would be nice to have.

Still, at the end of the day, if you want to be able to rely on
third-party tools like South, you might want to consider relying on less
magical fields. The more specialised and clever fields get, the less
likely South (or anything else) is going to be able to work out what's
going on with them. Generic Relations are kind of an unloved child
anyway within Django, which probably doesn't help - since most people
don't use them (including me), they don't get a lot of testing even in
their original form with South.

In summary, I'm working on it, but I can't promise any results. If you
want a guaranteed fix the only solution is from someone who is affected
by this and who has free time to dive in there and try to work out
what's wrong.

Andrew

[Stephen McDonald]

Thanks for the very thorough reply Andrew, that clears everything up for me. I had a bit of a chuckle at the generic relations comment.

What we'll do for now is recommend a full syncdb and migrate --fake for new installs (we actually have a createdb command that does this in one step), and we'll just create new migrations by hand without --auto from now on - only a slight inconvenience really, and at least it seems like this will work for now.

Thanks again.

Steve

 

Andrew

--
You received this message because you are subscribed to the Google Groups "South Users" group.
To post to this group, send email to south...@googlegroups.com.
To unsubscribe from this group, send email to south-users...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/south-users?hl=en.

[Martin Maney]

On Thu, Jul 05, 2012 at 08:26:19PM +0100, Andrew Godwin wrote:
> three years and try and considerably improve things. I can't promise
> this will lead to better handling of contribute_to_class fields, but
> I'll definitely keep it in mind - it would be nice to have.

I would beg a +1 on that, though I just want it not to stop working for
what I expect is a simpler case.

> going on with them. Generic Relations are kind of an unloved child
> anyway within Django, which probably doesn't help - since most people
> don't use them (including me), they don't get a lot of testing even in
> their original form with South.

I've always thought that so-called GenericForeignKey fields were an
abomination, myself. But I'm interested in this issue because I use a
trick for some simple custom fields which I do expect to work with
South, and as far as I can tell from some quick testing, they still do
work as of 0.7.5 (with Django 1.4). Here's the trick:

The custom field is AmountField, for "amounts" that are actually two
columns, the amount_value and the amount_units. This works very simply
with both Django and South by NOT injecting a field of type AmountField
in contribute_to_class, but rather.. well, like this:

def contribute_to_class(self, cls, name):
vfield = FloatField(**self.kwargs)
vfield.contribute_to_class(cls, name + '_value')
ufield = self.unit_field(embedded=True, **self.kwargs)
ufield.contribute_to_class(cls, name)
adesc = AmountDescriptor(name)
setattr(cls, name, adesc)

So both Django and South see only the injected fields, which are a
plain Float and... oh, right, I forgot about that indirection. The
unit_field is just a CharField with choices and some other non-db
options, and it uses the same trick to inject the CharField rather than
XxxUnitField, which is really just a source-level convenience -
syntactic sugar that could probably be handled by any sort of
compile-time macro system if Python had one.

The nice side effect is that, for custom fields that are just
parameterized versions of stock fields, it doesn't even need a trivial
introspection rule for South. It *is* just another CharField, etc.

This is also nice because it avoids the need, which I just recently
tripped over, of needing to keep obsolete custom fields around forever,
or at least as long as any migrations that have them frozen in are
liable to be run. The trick sidesteps that, so now I have another
reason to like it.

--
Self-pity can make one weep, as can onions. -- Fodor

[Stephen McDonald]

Just an update on this thread in case anyone comes across it:

--auto not working was actually a problem on our end. With the checks in place for the original issue described (dynamic fields being added) to not occur when south is doing its thing, we were actually (in error) omitting the super call to contribute_to_class for the main field in question. So to clarify:

- GenericForeignKey field exists on the model, with a contribute_to_class method

- contribute_to_class then dynamically adds extra fields to the model.

- The dynamically added fields causes "multiple assignments to same column" errors, so custom introspection rules pass in a "frozen_by_south" flag for south, and the fields aren't added in this case.

- We accidentally also omitted the super().contribute_to_class() call when the "frozen_by_south" flag was passed in, which cause --auto no longer work, as the main GenericForeignKey field where all this is implemented, would not exist on the model - reverting that got --auto to work again.

 

Thanks again.

Steve

 

Andrew

--
You received this message because you are subscribed to the Google Groups "South Users" group.
To post to this group, send email to south...@googlegroups.com.
To unsubscribe from this group, send email to south-users...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/south-users?hl=en.

--
Stephen McDonald
http://jupo.org