Code

Opened 7 years ago

Closed 7 years ago

#4625 closed (fixed)

NodeBase metaclass check logic is faulty

Reported by: (removed) Owned by: adrian
Component: Template system Version: master
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

As hinted at on the ml, the NodeBase.new code is a bit faulty-

    def __new__(cls, name, bases, attrs):
        """
        Ensures that either a 'render' or 'render_iter' method is defined on
        any Node sub-class. This avoids potential infinite loops at runtime.
        """
        if not (isinstance(attrs.get('render'), types.FunctionType) or
                isinstance(attrs.get('iter_render'), types.FunctionType)):
            raise TypeError('Unable to create Node subclass without either "render" or "iter_render" method.')
        return type.__new__(cls, name, bases, attrs)

Failings:
1) for code that has already broken the iter_render/render cycle, each derivative *still* has to assign a render/iter_render else it'll invalidly explode
2) no way to disable it (intermediate direct descendants from Node, say, adding helper functionality- they expect render/iter_render to be overriden, but the rules don't apply to them).

Attached is a patch that converts the check over to verifying the resultant (as in, full MRO) render/iter_render, and adds an option to specifically disable the verification where needed.

Attachments (2)

render-check-fix.patch (1.7 KB) - added by (removed) 7 years ago.
render-check-fix.2.patch (1.4 KB) - added by (removed) 7 years ago.
working version of the patch (using equality for the func check instead of identity)

Download all attachments as: .zip

Change History (3)

Changed 7 years ago by (removed)

Changed 7 years ago by (removed)

working version of the patch (using equality for the func check instead of identity)

comment:1 Changed 7 years ago by (removed)

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to fixed
  • Status changed from new to closed

Fixed via [5511] removal, already fixed in 'upstream' (ie, my branch), thus closing since it won't be reappearing.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.