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

2012-01-02 Thread Maurits van Rees

Op 02-01-12 12:09, Maurits van Rees schreef:

Now committed on trunk and branch 1.6.


BTW, http://svn.zope.org/repos/main/CMF/branches/2.2 uses 
Products.GenericSetup trunk.  Should that be changed to branch 1.6? 
Well, I have now committed this change as it seems reasonable.


GenericSetup trunk has an extra change in unicode and string handling in 
the exporters, which is probably safe, but I guess has a danger in it of 
being backwards incompatible for some instances, which is why the 
version has been increased to 1.7.x there.



--
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

___
Zope-CMF maillist  -  Zope-CMF@zope.org
https://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests


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

2012-01-02 Thread 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

___
Zope-CMF maillist  -  Zope-CMF@zope.org
https://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests


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

2011-12-26 Thread Maurits van Rees

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?


Kind regards,


--
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

___
Zope-CMF maillist  -  Zope-CMF@zope.org
https://mail.zope.org/mailman/listinfo/zope-cmf

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests