Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Zope: CMF

Re: SVN: Products.CMFDefault/trunk/Products/CMFDefault/__init__.py Move clashing imports into body of initialize().

  

 
Zope cmf RSS feed   Index | Next | Previous | View Threaded


y.2008 at wcm-solutions

Dec 28, 2008, 4:28 AM

Post #1 of 4 (627 views)
Permalink
Re: SVN: Products.CMFDefault/trunk/Products/CMFDefault/__init__.py Move clashing imports into body of initialize().
Hi Tres!


Tres Seaver wrote:
> Log message for revision 94389:
> Move clashing imports into body of initialize().
>
> Changed:
> U Products.CMFDefault/trunk/Products/CMFDefault/__init__.py

What are "clashing imports"? Can't they be fixed in an other way? I'd
prefer to have less code in initialize(), not more.


Maybe related:

Running CMF trunk tests with Zope 2.10, I now get random test-module
import failures like this one:

Module: Products.CMFActionIcons.tests.test_exportimport

Traceback (most recent call last):
File "..\Products\CMFActionIcons\tests\test_exportimport.py", line
24, in ?
from Products.CMFCore.testing import ExportImportZCMLLayer
File "..\Products\CMFCore\__init__.py", line 18, in ?
import PortalFolder
File "..\Products\CMFCore\PortalFolder.py", line 26, in ?
from App.class_init import default__class_init__ as InitializeClass
File "..\lib\python\App\class_init.py", line 16, in ?
import AccessControl.Permission
File "..\lib\python\AccessControl\Permission.py", line 18, in ?
import string, Products, Globals
File "..\lib\python\Globals\__init__.py", line 23, in ?
import Acquisition, ComputedAttribute, App.PersistentExtra, os
File "..\lib\python\App\PersistentExtra.py", line 24, in ?
from class_init import default__class_init__
ImportError: cannot import name default__class_init__


Digging a bit deeper, I found a circular import in Zope that causes
these failures:

AccessControl.Permission uses ApplicationDefaultPermissions defined in
App.class_init and App.class_init imports AccessControl.Permission.

AFAICS moving ApplicationDefaultPermissions to AccessControl.Permission
would be the best way to fix this.


Cheers,

Yuppie

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

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


tseaver at palladion

Dec 29, 2008, 9:13 AM

Post #2 of 4 (579 views)
Permalink
Re: SVN: Products.CMFDefault/trunk/Products/CMFDefault/__init__.py Move clashing imports into body of initialize(). [In reply to]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

yuppie wrote:

> Tres Seaver wrote:
>> Log message for revision 94389:
>> Move clashing imports into body of initialize().
>>
>> Changed:
>> U Products.CMFDefault/trunk/Products/CMFDefault/__init__.py
>
> What are "clashing imports"? Can't they be fixed in an other way? I'd
> prefer to have less code in initialize(), not more.

Importing anything at module scope in the __init__ can shadow the
"proper" import paths. I would rather do *nothing* at import time, and
then do any necessary imports inside the 'initialize', i.e., as late as
possible. An alternative would be to import 'initialize' itself from
another module.


> Maybe related:
>
> Running CMF trunk tests with Zope 2.10, I now get random test-module
> import failures like this one:
>
> Module: Products.CMFActionIcons.tests.test_exportimport
>
> Traceback (most recent call last):
> File "..\Products\CMFActionIcons\tests\test_exportimport.py", line
> 24, in ?
> from Products.CMFCore.testing import ExportImportZCMLLayer
> File "..\Products\CMFCore\__init__.py", line 18, in ?
> import PortalFolder
> File "..\Products\CMFCore\PortalFolder.py", line 26, in ?
> from App.class_init import default__class_init__ as InitializeClass
> File "..\lib\python\App\class_init.py", line 16, in ?
> import AccessControl.Permission
> File "..\lib\python\AccessControl\Permission.py", line 18, in ?
> import string, Products, Globals
> File "..\lib\python\Globals\__init__.py", line 23, in ?
> import Acquisition, ComputedAttribute, App.PersistentExtra, os
> File "..\lib\python\App\PersistentExtra.py", line 24, in ?
> from class_init import default__class_init__
> ImportError: cannot import name default__class_init__
>
>
> Digging a bit deeper, I found a circular import in Zope that causes
> these failures:

My bad: I hadn't tested with non-trunk Zopes. We might get away with
saying that CMF 2.2 requires at least Zope 2.11, but I think they would
break there too.

> AccessControl.Permission uses ApplicationDefaultPermissions defined in
> App.class_init and App.class_init imports AccessControl.Permission.
>
> AFAICS moving ApplicationDefaultPermissions to AccessControl.Permission
> would be the best way to fix this.

Yes, there is a bunch of stuff in App which doesn't belong there.
Unmasking it (by ripping out the imports from Globals) was a first step
to moving it into the right location.


Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 tseaver [at] palladion
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFJWQVJ+gerLs4ltQ4RAjCFAJ9iaQEdqXOg51vtEbkVtLdA7ibeogCePSQs
b1DvuGnFKL+s9LkLGHDHdNw=
=ByOP
-----END PGP SIGNATURE-----

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

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


y.2008 at wcm-solutions

Dec 29, 2008, 11:35 AM

Post #3 of 4 (582 views)
Permalink
Re: SVN: Products.CMFDefault/trunk/Products/CMFDefault/__init__.py Move clashing imports into body of initialize(). [In reply to]
Hi Tres!


Tres Seaver wrote:
> yuppie wrote:
>
>> Tres Seaver wrote:
>>> Log message for revision 94389:
>>> Move clashing imports into body of initialize().
>>>
>>> Changed:
>>> U Products.CMFDefault/trunk/Products/CMFDefault/__init__.py
>> What are "clashing imports"? Can't they be fixed in an other way? I'd
>> prefer to have less code in initialize(), not more.
>
> Importing anything at module scope in the __init__ can shadow the
> "proper" import paths. I would rather do *nothing* at import time, and
> then do any necessary imports inside the 'initialize', i.e., as late as
> possible. An alternative would be to import 'initialize' itself from
> another module.

Well. The initialize() pattern is Zope 2 Products specific. I'm fine
with moving everything that isn't useful outside initialize() into that
function.

But some imports in __init__ have (also) a different purpose: They make
sure security is initialized correctly. Moving those imports inside
initialize() makes it less predictable when security is initialized.

>> Running CMF trunk tests with Zope 2.10, I now get random test-module
>> import failures like this one:
>>
[...]
>>
>> Digging a bit deeper, I found a circular import in Zope that causes
>> these failures:
>
> My bad: I hadn't tested with non-trunk Zopes. We might get away with
> saying that CMF 2.2 requires at least Zope 2.11, but I think they would
> break there too.

Yes. I see the same issue with Zope 2.11.

I propose to fix this in Zope 2.10 and 2.11. And to make the latest
Version of Zope 2.10 or 2.11 required for CMF 2.2.

>> AccessControl.Permission uses ApplicationDefaultPermissions defined in
>> App.class_init and App.class_init imports AccessControl.Permission.
>>
>> AFAICS moving ApplicationDefaultPermissions to AccessControl.Permission
>> would be the best way to fix this.
>
> Yes, there is a bunch of stuff in App which doesn't belong there.
> Unmasking it (by ripping out the imports from Globals) was a first step
> to moving it into the right location.

Do you want to change that as part of your Zope refactoring or should I
check in my fix?


Cheers,

Yuppie

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

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


tseaver at palladion

Dec 29, 2008, 12:12 PM

Post #4 of 4 (581 views)
Permalink
Re: SVN: Products.CMFDefault/trunk/Products/CMFDefault/__init__.py Move clashing imports into body of initialize(). [In reply to]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

yuppie wrote:
> Hi Tres!
>
>
> Tres Seaver wrote:
>> yuppie wrote:
>>
>>> Tres Seaver wrote:
>>>> Log message for revision 94389:
>>>> Move clashing imports into body of initialize().
>>>>
>>>> Changed:
>>>> U Products.CMFDefault/trunk/Products/CMFDefault/__init__.py
>>> What are "clashing imports"? Can't they be fixed in an other way? I'd
>>> prefer to have less code in initialize(), not more.
>> Importing anything at module scope in the __init__ can shadow the
>> "proper" import paths. I would rather do *nothing* at import time, and
>> then do any necessary imports inside the 'initialize', i.e., as late as
>> possible. An alternative would be to import 'initialize' itself from
>> another module.
>
> Well. The initialize() pattern is Zope 2 Products specific. I'm fine
> with moving everything that isn't useful outside initialize() into that
> function.
>
> But some imports in __init__ have (also) a different purpose: They make
> sure security is initialized correctly. Moving those imports inside
> initialize() makes it less predictable when security is initialized.
>
>>> Running CMF trunk tests with Zope 2.10, I now get random test-module
>>> import failures like this one:
>>>
> [...]
>>> Digging a bit deeper, I found a circular import in Zope that causes
>>> these failures:
>> My bad: I hadn't tested with non-trunk Zopes. We might get away with
>> saying that CMF 2.2 requires at least Zope 2.11, but I think they would
>> break there too.
>
> Yes. I see the same issue with Zope 2.11.
>
> I propose to fix this in Zope 2.10 and 2.11. And to make the latest
> Version of Zope 2.10 or 2.11 required for CMF 2.2.

Sounds fine.

>>> AccessControl.Permission uses ApplicationDefaultPermissions defined in
>>> App.class_init and App.class_init imports AccessControl.Permission.
>>>
>>> AFAICS moving ApplicationDefaultPermissions to AccessControl.Permission
>>> would be the best way to fix this.
>> Yes, there is a bunch of stuff in App which doesn't belong there.
>> Unmasking it (by ripping out the imports from Globals) was a first step
>> to moving it into the right location.
>
> Do you want to change that as part of your Zope refactoring or should I
> check in my fix?

I think we could put your fix in on the Zope 2.10 and 2.11 branches, as
well as on my no_globals_imports branch. I'm not ready to commit to the
other cleanups right now.


Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 tseaver [at] palladion
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFJWS8t+gerLs4ltQ4RAqivAKDKnh4CHnUT6ztd1uFTMUpBedOgwQCfT1Ou
1tA0QihSVKrUHBvkxR/dPew=
=sDtT
-----END PGP SIGNATURE-----

_______________________________________________
Zope-CMF maillist - Zope-CMF [at] lists
http://mail.zope.org/mailman/listinfo/zope-cmf

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

Zope cmf RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact lists@gossamer-threads.com
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.