[Sqlalchemy-tickets] Issue #3267: `__declare_last__` and multiple mixins (zzzeek/sqlalchemy)
Brought to you by:
zzzeek
|
From: Jack Z. <iss...@bi...> - 2014-12-07 04:28:21
|
New issue 3267: `__declare_last__` and multiple mixins https://bitbucket.org/zzzeek/sqlalchemy/issue/3267/__declare_last__-and-multiple-mixins Jack Zhou: Currently, declarative appears to only support a single mixin with `__declare_last__` defined. The relevant code from 0.9.8: ``` #!python for base in cls.__mro__: _is_declarative_inherits = hasattr(base, '_decl_class_registry') if '__declare_last__' in base.__dict__: @event.listens_for(mapper, "after_configured") def go(): cls.__declare_last__() if '__declare_first__' in base.__dict__: @event.listens_for(mapper, "before_configured") def go(): cls.__declare_first__() ``` As you can see, even though it inspects each class in the MRO, declarative calls `cls.__declare_last__`, which happens to be the first base class in the MRO that defines `__declare_last__`; it also calls it multiple times, as many times as there are bases that define `__declare_first__`. (master seems to have fixed the multiple calls issue, but is still limited to a single `__declare_last__`.) I think it's better to allow each base class in the MRO to define its own `__declare_last__`: ``` #!python for base in cls.__mro__: _is_declarative_inherits = hasattr(base, '_decl_class_registry') if '__declare_last__' in base.__dict__: def closure(base): @event.listens_for(mapper, "after_configured") def go(): base.__dict__["__declare_last__"].__func__(cls) closure(base) if '__declare_first__' in base.__dict__: def closure(base): @event.listens_for(mapper, "before_configured") def go(): base.__dict__["__declare_first__"].__func__(cls) closure(base) ``` To me, this is more intuitive and useful for mixins. One downside I see is not being able to override the behavior of a mixin in a base class, which can potentially be troublesome. If you want to go with the multiple `__declare_last__` approach, I'd be glad to provide a patch for master. |