[Zope-CMF] RFC: Being more forgiving when importing toolset.xml

[Maurits van Rees]

Op 26-12-11 23:56, Maurits van Rees schreef:
> Hi,
>
>
> The problem
> -----------
>
> The toolset registry of portal_setup can get out of touch with reality:
> a product may have installed a tool a while ago and made it required,
> but meanwhile the product author has unthinkingly renamed the class or
> module of the tool; or the product has been removed from the buildout
> without being uninstalled; or the uninstall does not take care of
> cleaning up the required tools from the registry.
>
> Now the admin of this website installs a second product which is totally
> unrelated. It has its own required tool, which it registers in
> toolset.xml. On install, the admin gets a traceback:
>
> Module Products.GenericSetup.tool, line 123, in importToolset
> TypeError: 'NoneType' object is not callable
>
> This is because the tool of the first product is still in the required
> tools list. The admin does not understand this and files a bug report
> for the second product as it cannot be installed. Sample bug reports are
> here:
>
> http://plone.org/products/quintagroup-dropdownmenu/issues/4
> http://plone.org/products/simplealias/issues/6
> http://plone.org/products/ploneglossary/issues/13
>
> And a thread on how to fix it when it has already gone wrong:
> http://comments.gmane.org/gmane.comp.web.zope.plone.devel/26348
>
>
> The proposal
> ------------
>
> I propose to not break in this case, but give a warning and continue
> with the next tool, much like was already done for missing import steps.
>
> This should lower the number of sites where installs of new products
> fail for a reason that has nothing to do with that product. It should
> consequently also bring down the number of misdirected support requests.
>
> Downside could be that when you are developing a new product and make a
> typo in the class name in toolset.xml it does not break anymore but only
> print a warning message that may be easily overlooked.
>
> I would say the benefit outweighs the downside.
>
>
> The code
> --------
>
> Currently the code in Products/GenericSetup/tool.py starts like this:
>
> for info in toolset.listRequiredToolInfo():
> tool_id = str(info['id'])
> tool_class = _resolveDottedName(info['class'])
> if tool_class is None:
> logger.info('Class %(class)s not found for '
> 'tool %(id)s' % info)
> # code that may break when tool_class is None
> ...
>
>
> The code that is executed in the 'tool_class is None' condition should
> become:
>
> logger.warning("Not creating required tool %(id)s, because "
> "class %(class)s is not found." % info)
> continue
>
>
> Tests on CMF trunk and CMF 2.2 pass with this.
>
> Would this change be acceptable on trunk and branch 1.6?

Now committed on trunk and branch 1.6.


-- 
Maurits van Rees   http://maurits.vanrees.org/
Web App Programmer at Zest Software: http://zestsoftware.nl
"Logical thinking shows conclusively that logical thinking
is inconclusive." - My summary of Gödel, Escher, Bach