On 04/04/2012 06:08 PM, Ethan Jucovy wrote:
Ah, thanks for pointing that out. I was about to write a ticket to get
someone to get the theme engine to change the implementation.
I don't particularly mind that a plugin wants to do a lot in their
__init__ as long as they are prepared to deal with the potential error. As
I think I already say, the only fault I see in Trac on this issue is that
it doesn't deal with it quite helpfully enough. Basically I think it should
survive the event to the extent of allowing the admin to disable the
offending plugin from the admin interface.
Yeah, I just wanted to bring this up with a reference ASAP to make sure
it's easy to revisit the core patch later. I agree that Trac should deal
with this more helpfully. Surviving the exception would be nice; even just
catching the exception and re-raising it wrapped in some kind of
PluginInitializationException(plugin_name, exc) would probably be better
than the current behavior.
FWIW this should probably apply to all plugin __init__ errors; the infinite
recursion error just happens to be harder to debug than most since
understanding it relies on some knowledge of Trac internals. But I notice
for example that code like the following in an active plugin is not handled
gracefully either:
class BadPlugin(Component):
implements(IRequestFilter)
def __init__(self, *args, **kw):
raise Exception
Best,
Ethan
OK, I think we are in agreement but you make some good points that I
updated https://issues.apache.org/bloodhound/ticket/27 with.
I'm almost tempted to adjust the patch to only apply to the
ThemeEnginePlugin and investigate raising an appropriate exception as
suggested. What does everyone else think?
Cheers,
Gary
