django-mptt 0.5 breaks the CMS

[ojii]

MPTT meta attrs (eg 'parent_attr') are now on Model._mptt_meta not on Model._meta. Awesome that noone gives a shit about backwards compatiblity.

See https://github.com/django-mptt/django-mptt/compare/0.4.2...0.5.0#diff-38

DDN: Do we support 0.5 in 2.2.1 or is this 2.3 material?

[jasongonzales23]

Hi,

I was having an issue with cms 2.2 and saw this issue. The issue was plugins not loading in the cms. Once I downgraded to 0.4.2 the issue was fixed. Sad face emoticon here.

[craigds]

Seems like it should be pretty trivial to fix? _mptt_meta has been in mptt since 0.4, so as long as you're requiring mptt>=0.4 you should just be able to rename _meta to _mptt_meta. They're usually not the sort of thing people will be overriding in custom Page subclasses either.

The _meta copies were removed in 0.5 since they had been deprecated for over a year, and that seemed plenty long enough.

[ojii]

Why then, if this was a "deprecation over a year" has this never caused deprecation warnings?! What tells me they won't break compatibility in 0.6 again? I can't make a release every time they do.

My thoughts on this are on https://groups.google.com/forum/#!topic/django-cms-developers/S3jc69mug7A

[craigds]

You're right, it should have got a deprecation warning. I didn't realise it didn't. I'll release 0.5.1 today with the attributes back (with deprecation warnings to be removed in 0.6)

It was, however in the upgrade notes http://django-mptt.github.com/django-mptt/upgrade.html#meta-attributes-moved-to-mptt-meta

[ojii]

@craigds, are you the maintainer of django-mptt?

[craigds]

waves hand yes I am

[ojii]

alright, anything else we should know for future releases?

How can we prevent these things from happening again? (We had the same kind of problem with 0.4)

We can't possibly release as often as you do, so we'll need some sort of slower deprecation path whenever possible. Our current release cycle is longer than half a year...

[craigds]

Actually, I've stumbled across maybe why the deprecation warnings weren't added in the first place. I don't know how to do them. getattr, property() and descriptors only work on classes, not on instances, and _meta is an instance of Options. And I can't change the django Options class, obviously.

The only way I can think of is to set cls._meta.class to something else that takes care of deprecation warnings, but that seems too dodgy to be worth it.

[craigds]

Future releases: I really don't have any plans for 0.6 at this stage. mptt is pretty much feature complete AFAIK, so I'm going to wait and see what issues people file. It's been a year since 0.4.2 and it will probably be another year till 0.6.

I took over maintaining mptt between 0.3 and 0.4 to try and unify the millions of forks into something coherent. It had been abandoned for a couple of years at that stage. 0.4 was a huge release as I had to overhaul most of the codebase. So naturally there were a few breaking changes.

0.5 is not nearly as big, and I don't even know what to put in 0.6. The point of the changes in 0.4 and 0.5 was to clean up some of the cruft that's been sitting around for years. I'm pretty happy with it now, so I don't expect anything much to change in 0.6.

The only things that will be removed in 0.6 are:

• the .tree TreeManager, and tree_manager_attr which currently have DeprecationWarnings to that effect.
• some of the hacks that make mptt work with django 1.1, since I don't want to support 1.1 anymore.

From a brief glance at the django-cms code, it looks like neither of those will affect you.

[craigds]

Just to be clear, since I can't find a way to add deprecation warnings, cancel that 0.5.1. You'll have to change your references to _meta to _mptt_meta.

[ojii]

Couldn't you add them using descriptors?

Something like:

import warnings

class DeprecationProxyDescriptor(object):
    def __init__(self, attr, target, message):
        self.attr = attr
        self.message = message
        self.target = target
    def __get__(self, instance, instance_type=None):
        if not instance:
            raise AttributeError("%r can only be accessed from instances and is deprecated (%s)" % (self.attr, self.message))
        # don't actually use (Pending)DeprecationWarning, since it's ignored by default
        warnings.warn(self.message) 
        return getattr(self.target, self.attr)

    def __set__(self, instance, value):
        # don't actually use (Pending)DeprecationWarning, since it's ignored by default
        warnings.warn(self.message) 
        setattr(self.target, self.attr, value)

    def __delete__(self, instance):
        # don't actually use (Pending)DeprecationWarning, since it's ignored by default
        warnings.warn(self.message) 
        delattr(self.target, self.attr, value)

[craigds]

Yeah, that's what I tried first, but descriptors only seem to work when they're methods on the class, not created afterwards on the instance:

In [1]: class Options(object):
   ...:   pass
   ...: 

In [2]: _meta = Options()

In [3]: class Descriptor(object):
   ...:   def __get__(self, instance, type=None):
   ...:     print instance, type
   ...:     return "hi"
   ...: 

In [4]: setattr(_meta, "tree_id", Descriptor())

In [5]: _meta.tree_id
Out[5]: <__main__.Descriptor at 0x10ca67410>

[ojii]

You can add it to _meta.__class__ since the class itself is also unique to the model

[craigds]

It's not actually, _meta is not actually an instance of the Meta class you define on your model.

It's an instance of django.db.models.base.Options

[ojii]

Okay so let's forget about that. If you could release a 0.5.1 that's compatible with our 2.2, I'll make 2.2.1 switch to the new meta attribute, problem solved.

[craigds]

I don't see any point in just putting the _meta copies back without deprecation warnings. That will just put the problem off till 0.6 and we'll have to go through this again. And we'll have to have docs saying they weren't in 0.5 but were back again in 0.5.1 but were removed again in 0.6. Messy.

Now that they're removed I'd prefer to just leave them removed.

The solution for users of your 2.2 would just be to not upgrade to mptt 0.5 just yet, but presumably if 2.2.1 is around the corner that's not such a big deal.

[ojii]

Well 'around the corner' = potentially weeks/months. So it's soon in Valve Time [1]

[1] http://developer.valvesoftware.com/wiki/Valve_Time

[craigds]

I've checked in https://github.com/django-mptt/django-mptt/d3d1556e218b46a652951e61140c5ec6260d07c1 to try and resolve this. I decided to just change _meta.class so I could provide deprecation warnings.

Just waiting on a response to https://github.com/django-mptt/django-mptt/issues/153 , then I'll release as 0.5.1.

[craigds]

Released django-mptt 0.5.1: https://groups.google.com/forum/#!topic/django-mptt-dev/gkGSSArXfx4

That should fix this issue for 2.2.0; please also rename the attributes for future releases so they will work with mptt 0.6.

Cheers

[ojii]

it will be fixed to 0.5.1 for now