Re: [Zope-CMF] SVN: Products.GenericSetup/trunk/ Refactored global registries to use global named utilities.

2011-03-11 Thread yuppie 
Hi Godefroid!


A few questions:

Godefroid Chapelle wrote:
 Log message for revision 120850:
Refactored global registries to use global named utilities.

merge of branch gotcha-registries-use-utilities
[...]
 Modified: Products.GenericSetup/trunk/Products/GenericSetup/registry.py
 ===
 --- Products.GenericSetup/trunk/Products/GenericSetup/registry.py 
 2011-03-10 15:13:48 UTC (rev 120849)
 +++ Products.GenericSetup/trunk/Products/GenericSetup/registry.py 
 2011-03-10 16:56:57 UTC (rev 120850)
[...]
 @@ -721,13 +758,26 @@
   # metadata.xml description trumps ZCML description... awkward
   info.update( metadata )

 -self._profile_info[ profile_id ] = info
 +sm.registerUtility(info, provided=IProfile, name=profile_id)

 +def _computeProfileId(self, name, product):
 +profile_id = '%s:%s' % (product or 'other', name)
 +return profile_id
 +
 +security.declareProtected( ManagePortal, 'unregisterProfile' )
 +def unregisterProfile( self, name, product=None):
 +profile_id = self._computeProfileId(name, product)
 +sm = getGlobalSiteManager()
 +sm.unregisterUtility(provided=IProfile, name=profile_id)
 +
   security.declarePrivate( 'clear' )
   def clear( self ):
 +sm = getGlobalSiteManager()
 +profile_ids = [profile_id for profile_id, profile_info
 +in sm.getUtilitiesFor(IProfile)]
 +for profile_id in profile_ids:
 +sm.unregisterUtility(provided=IProfile, name=profile_id)

 -self._profile_info = {}
 -self._profile_ids = []

Does GlobalRegistryStorage not work for the ProfileRegistry?


 Modified: 
 Products.GenericSetup/trunk/Products/GenericSetup/tests/test_registry.py
 ===
 --- Products.GenericSetup/trunk/Products/GenericSetup/tests/test_registry.py  
 2011-03-10 15:13:48 UTC (rev 120849)
 +++ Products.GenericSetup/trunk/Products/GenericSetup/tests/test_registry.py  
 2011-03-10 16:56:57 UTC (rev 120850)
 @@ -1014,6 +1014,8 @@
 , ConformsToIProfileRegistry
 ):

 +
 +
   def _getTargetClass( self ):

   from Products.GenericSetup.registry import ProfileRegistry
 @@ -1045,7 +1047,7 @@
   , PRODUCT
   , PROFILE_TYPE
   )
 -
 +
   self.assertEqual( len( registry.listProfiles() ), 1 )
   self.assertEqual( len( registry.listProfileInfo() ), 1 )

You touched test_registry.py just to add some extra whitespace in 
strange places?


 Modified: Products.GenericSetup/trunk/Products/GenericSetup/zcml.py
 ===
 --- Products.GenericSetup/trunk/Products/GenericSetup/zcml.py 2011-03-10 
 15:13:48 UTC (rev 120849)
 +++ Products.GenericSetup/trunk/Products/GenericSetup/zcml.py 2011-03-10 
 16:56:57 UTC (rev 120850)
[...]
   def cleanUpImportSteps():
 -global _import_step_regs
 -for name in _import_step_regs:
 -try:
 -_import_step_registry.unregisterStep(name)
 -except KeyError:
 -pass
 +pass

 -_import_step_regs = []
 -
   def cleanUpExportSteps():
 -global _export_step_regs
 -for name in _export_step_regs:
 -try:
 -_export_step_registry.unregisterStep(name)
 -except KeyError:
 -pass
 +pass

Why didn't you remove cleanUpImportSteps and cleanUpExportSteps?


Cheers,

Yuppie
___
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] SVN: Products.GenericSetup/trunk/ Refactored global registries to use global named utilities.

2011-03-11 Thread Godefroid Chapelle 
Hi Yvo,

Thanks for reviewing my work !

Le 11/03/11 10:43, yuppie a écrit :
 Hi Godefroid!


 A few questions:

 Godefroid Chapelle wrote:
 Log message for revision 120850:
 Refactored global registries to use global named utilities.

 merge of branch gotcha-registries-use-utilities
 [...]
 Modified: Products.GenericSetup/trunk/Products/GenericSetup/registry.py
 ===
 --- Products.GenericSetup/trunk/Products/GenericSetup/registry.py
 2011-03-10 15:13:48 UTC (rev 120849)
 +++ Products.GenericSetup/trunk/Products/GenericSetup/registry.py
 2011-03-10 16:56:57 UTC (rev 120850)
 [...]
 @@ -721,13 +758,26 @@
 # metadata.xml description trumps ZCML description... awkward
 info.update( metadata )

 - self._profile_info[ profile_id ] = info
 + sm.registerUtility(info, provided=IProfile, name=profile_id)

 + def _computeProfileId(self, name, product):
 + profile_id = '%s:%s' % (product or 'other', name)
 + return profile_id
 +
 + security.declareProtected( ManagePortal, 'unregisterProfile' )
 + def unregisterProfile( self, name, product=None):
 + profile_id = self._computeProfileId(name, product)
 + sm = getGlobalSiteManager()
 + sm.unregisterUtility(provided=IProfile, name=profile_id)
 +
 security.declarePrivate( 'clear' )
 def clear( self ):
 + sm = getGlobalSiteManager()
 + profile_ids = [profile_id for profile_id, profile_info
 + in sm.getUtilitiesFor(IProfile)]
 + for profile_id in profile_ids:
 + sm.unregisterUtility(provided=IProfile, name=profile_id)

 - self._profile_info = {}
 - self._profile_ids = []

 Does GlobalRegistryStorage not work for the ProfileRegistry?

Good point.

Done ! I did it for UpgradeRegistry as well.



 Modified:
 Products.GenericSetup/trunk/Products/GenericSetup/tests/test_registry.py
 ===
 ---
 Products.GenericSetup/trunk/Products/GenericSetup/tests/test_registry.py
 2011-03-10 15:13:48 UTC (rev 120849)
 +++
 Products.GenericSetup/trunk/Products/GenericSetup/tests/test_registry.py
 2011-03-10 16:56:57 UTC (rev 120850)
 @@ -1014,6 +1014,8 @@
 , ConformsToIProfileRegistry
 ):

 +
 +
 def _getTargetClass( self ):

 from Products.GenericSetup.registry import ProfileRegistry
 @@ -1045,7 +1047,7 @@
 , PRODUCT
 , PROFILE_TYPE
 )
 -
 +
 self.assertEqual( len( registry.listProfiles() ), 1 )
 self.assertEqual( len( registry.listProfileInfo() ), 1 )

 You touched test_registry.py just to add some extra whitespace in
 strange places?

Sorry for the noise. Do you want a revert ?



 Modified: Products.GenericSetup/trunk/Products/GenericSetup/zcml.py
 ===
 --- Products.GenericSetup/trunk/Products/GenericSetup/zcml.py
 2011-03-10 15:13:48 UTC (rev 120849)
 +++ Products.GenericSetup/trunk/Products/GenericSetup/zcml.py
 2011-03-10 16:56:57 UTC (rev 120850)
 [...]
 def cleanUpImportSteps():
 - global _import_step_regs
 - for name in _import_step_regs:
 - try:
 - _import_step_registry.unregisterStep(name)
 - except KeyError:
 - pass
 + pass

 - _import_step_regs = []
 -
 def cleanUpExportSteps():
 - global _export_step_regs
 - for name in _export_step_regs:
 - try:
 - _export_step_registry.unregisterStep(name)
 - except KeyError:
 - pass
 + pass

 Why didn't you remove cleanUpImportSteps and cleanUpExportSteps?

I should have.

All cleanup functions are now gone.



 Cheers,

 Yuppie


-- 
Godefroid Chapelle (aka __gotcha) http://bubblenet.be
___
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