Re: SVN: Zope/branches/ajung-zpt-end-game/lib/python/Products/PageTemplates/tests/testZopePageTemplate.py A recent fix in the zope 3 traversing code now requires the ITraversable
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Philipp von Weitershausen wrote:
Log message for revision 68328: A recent fix in the zope 3 traversing code now requires the ITraversable adapter to be present even when traversing just dicts. No problem, we can set it up quite easily.
This basically finishes this branch. All tests pass, some 3rd party apps have been tested with it. There are a few test failures in the CMF and others (e.g. Plone), but that's mostly "their fault" (incorrect or unexpected usage of certain things).
Those are BBB fouls, as those failures do not emit deprecation warnings when run against 2.9. In fact, none of the deprecation warnings which are raised when running the CMF 2.0 tests under 2.9 allow for failures under 2.10. The failrures are mostly instances of: - The "empty path element" exception. Zope3's choice to make that an error is undefended, AFAIK (Fred never replied to your query about the reason), and needs BBB in any case. - CMFDefault/skin/configure.zcml blows up because the syntax of the 'browser:skin' directive has changed in a BBB-incompatible way. The now-failing directive is:: <browser:skin name="cmf" layers="cmf default" /> and the traceback is *very* weird:: ZopeXMLConfigurationError: File \ "/tmp/endgame/Products/CMFDefault/skin/configure.zcml", line \ 10.2-13.8 ConfigurationError: ('Invalid value for', 'layers', "ImportError:\ Couldn't import default, No module named default in cmf default") - "No traversable adapter found" raised out of 'boboAwareZopeTraverse'. This one is likely due to the prior ZCML issue, which blows out ZCML processing early. These problems aren't blockers for a 2.10 beta, but they must be resolved before a final release. Tres. - -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFEelcj+gerLs4ltQ4RAqRcAJ0feXoFF2V19kS0VBRuwmW4BDkGFQCeM8Ta VRMWb2tyUuWSSzL1EtM80bk= =7/6v -----END PGP SIGNATURE-----
--On 28. Mai 2006 22:06:27 -0400 Tres Seaver <tseaver@palladion.com> wrote:
These problems aren't blockers for a 2.10 beta, but they must be resolved before a final release.
I agree that these issues should be fixed. But it is now time to get 2.10 beta 1 out of the door as soon as possible. We are already running very late however Philipp did a *really* tremendous job finalizing the ZPT integration and the delay was/is worth the effort (independent of our primary goal to couple the Zope 2 and Zope 3 releases. -aj -- ZOPYX Ltd. & Co. KG - Charlottenstr. 37/1 - 72070 Tübingen - Germany Web: www.zopyx.com - Email: info@zopyx.com - Phone +49 - 7071 - 793376 E-Publishing, Python, Zope & Plone development, Consulting
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Andreas Jung wrote:
--On 28. Mai 2006 22:06:27 -0400 Tres Seaver <tseaver@palladion.com> wrote:
These problems aren't blockers for a 2.10 beta, but they must be resolved before a final release.
I agree that these issues should be fixed. But it is now time to get 2.10 beta 1 out of the door as soon as possible. We are already running very late however Philipp did a *really* tremendous job finalizing the ZPT integration and the delay was/is worth the effort (independent of our primary goal to couple the Zope 2 and Zope 3 releases.
Agreed. Thank you both for your efforts here. Tres. - -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFEexsG+gerLs4ltQ4RAv2CAKDOLUu3im49Fbrn8sn7WvfWjvocMQCfYDm4 J3QmxQFwbnkIHXOMA0H4gQY= =T1BU -----END PGP SIGNATURE-----
Tres Seaver wrote:
Philipp von Weitershausen wrote:
Log message for revision 68328: A recent fix in the zope 3 traversing code now requires the ITraversable adapter to be present even when traversing just dicts. No problem, we can set it up quite easily.
This basically finishes this branch. All tests pass, some 3rd party apps have been tested with it. There are a few test failures in the CMF and others (e.g. Plone), but that's mostly "their fault" (incorrect or unexpected usage of certain things).
Those are BBB fouls, as those failures do not emit deprecation warnings when run against 2.9. In fact, none of the deprecation warnings which are raised when running the CMF 2.0 tests under 2.9 allow for failures under 2.10.
The failrures are mostly instances of:
- The "empty path element" exception. Zope3's choice to make that an error is undefended, AFAIK (Fred never replied to your query about the reason), and needs BBB in any case.
Right, Fred didn't reply, but we managed to have a little chat. After this I replied to your email: http://mail.zope.org/pipermail/zope-dev/2006-May/027493.html Basically: * we hardly see a use case for empty path elements. They don't work right now, at least not in the way that you expected them to work. So, we're not really breaking anything if we take away something that didn't work in the first place. * AFAIK the CMF tries to compile empty TALES expessions (I don't know yet whether only in tests or also in "real" code, Hanno Schlichting has been testing my branch against the CMF in the past days). This isn't a use case either. It's just misguided usage, as I would call it (Fred agrees). We can fix the CMF to simply not compile empty TALES expressions.
- CMFDefault/skin/configure.zcml blows up because the syntax of the 'browser:skin' directive has changed in a BBB-incompatible way. The now-failing directive is::
<browser:skin name="cmf" layers="cmf default" />
and the traceback is *very* weird::
ZopeXMLConfigurationError: File \ "/tmp/endgame/Products/CMFDefault/skin/configure.zcml", line \ 10.2-13.8 ConfigurationError: ('Invalid value for', 'layers', "ImportError:\ Couldn't import default, No module named default in cmf default")
Ah yes. This is due to a missing ZCML declaration in Five. I already fixed it on the Five trunk (http://mail.zope.org/pipermail/checkins/2006-May/001859.html), but the zpt branch is a bit too old to have this fix. That's why it works fine on the Zope trunk but not on the zpt branch.
- "No traversable adapter found" raised out of 'boboAwareZopeTraverse'. This one is likely due to the prior ZCML issue, which blows out ZCML processing early.
Very likely. The correct traversable adapter is set up when you load Five/traversing.zcml or zope.traversing/configure.zcml. If the ZCML machinery doesn't get far enough, you don't have the adapter. Philipp
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Philipp von Weitershausen wrote:
Tres Seaver wrote:
Philipp von Weitershausen wrote:
Log message for revision 68328: A recent fix in the zope 3 traversing code now requires the ITraversable adapter to be present even when traversing just dicts. No problem, we can set it up quite easily.
This basically finishes this branch. All tests pass, some 3rd party apps have been tested with it. There are a few test failures in the CMF and others (e.g. Plone), but that's mostly "their fault" (incorrect or unexpected usage of certain things).
Those are BBB fouls, as those failures do not emit deprecation warnings when run against 2.9. In fact, none of the deprecation warnings which are raised when running the CMF 2.0 tests under 2.9 allow for failures under 2.10.
The failrures are mostly instances of:
- The "empty path element" exception. Zope3's choice to make that an error is undefended, AFAIK (Fred never replied to your query about the reason), and needs BBB in any case.
Right, Fred didn't reply, but we managed to have a little chat. After this I replied to your email: http://mail.zope.org/pipermail/zope-dev/2006-May/027493.html
Basically:
* we hardly see a use case for empty path elements. They don't work right now, at least not in the way that you expected them to work. So, we're not really breaking anything if we take away something that didn't work in the first place.
The CMF (and Plone on top of it) work *fine* as they are today for completely empty expressions. Reverting the BBB-incompatible cahnge, and adding deprecation warnings, is our standard practice here. The "empty element" (as opposed to "empty expression", which is what CMF / Plone use) hasn't worked in qute a while, AFAICT. 'unrestrictedTraverse' raises a KeyError in both 2.8 and 2.9.
* AFAIK the CMF tries to compile empty TALES expessions (I don't know yet whether only in tests or also in "real" code, Hanno Schlichting has been testing my branch against the CMF in the past days). This isn't a use case either. It's just misguided usage, as I would call it (Fred agrees). We can fix the CMF to simply not compile empty TALES expressions.
Yes, but you can't break the CMF in a release without doing the deprecation dance first.
- CMFDefault/skin/configure.zcml blows up because the syntax of the 'browser:skin' directive has changed in a BBB-incompatible way. The now-failing directive is::
<browser:skin name="cmf" layers="cmf default" />
and the traceback is *very* weird::
ZopeXMLConfigurationError: File \ "/tmp/endgame/Products/CMFDefault/skin/configure.zcml", line \ 10.2-13.8 ConfigurationError: ('Invalid value for', 'layers', "ImportError:\ Couldn't import default, No module named default in cmf default")
Ah yes. This is due to a missing ZCML declaration in Five. I already fixed it on the Five trunk (http://mail.zope.org/pipermail/checkins/2006-May/001859.html), but the zpt branch is a bit too old to have this fix. That's why it works fine on the Zope trunk but not on the zpt branch.
They I guess we should update the external for Five on the branch, and verify?
- "No traversable adapter found" raised out of 'boboAwareZopeTraverse'. This one is likely due to the prior ZCML issue, which blows out ZCML processing early.
Very likely. The correct traversable adapter is set up when you load Five/traversing.zcml or zope.traversing/configure.zcml. If the ZCML machinery doesn't get far enough, you don't have the adapter.
Yup. Tres. - -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFEexrF+gerLs4ltQ4RArJhAJ9F0sRAGJQPCuw9nQj/LhB6PAqLAACgouNG z7AJL0Z+2susI9X0L2vPiJE= =vMph -----END PGP SIGNATURE----- -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com
Tres Seaver wrote:
Basically:
* we hardly see a use case for empty path elements. They don't work right now, at least not in the way that you expected them to work. So, we're not really breaking anything if we take away something that didn't work in the first place.
The CMF (and Plone on top of it) work *fine* as they are today for completely empty expressions. Reverting the BBB-incompatible cahnge, and adding deprecation warnings, is our standard practice here.
Yes, for changes that ripped out well-understood and well-supported features. I don't know whether we have a standard practice for cases like this, but there's definitely a straight-forward practice: fix the CMF :).
The "empty element" (as opposed to "empty expression", which is what CMF / Plone use) hasn't worked in qute a while, AFAICT. 'unrestrictedTraverse' raises a KeyError in both 2.8 and 2.9.
* AFAIK the CMF tries to compile empty TALES expessions (I don't know yet whether only in tests or also in "real" code, Hanno Schlichting has been testing my branch against the CMF in the past days). This isn't a use case either. It's just misguided usage, as I would call it (Fred agrees). We can fix the CMF to simply not compile empty TALES expressions.
Yes, but you can't break the CMF in a release without doing the deprecation dance first.
Or we could simply fix the CMF. It would have to be done at some point anyways, only that if we do it now it limits the effort. To provide proper BBB for weird behaviour that doesn't even have a good use case is a much larger effort. CMF bugfix releases could be on their way before Zope 2.10 hits even final.
- CMFDefault/skin/configure.zcml blows up because the syntax of the 'browser:skin' directive has changed in a BBB-incompatible way. The now-failing directive is::
<browser:skin name="cmf" layers="cmf default" />
and the traceback is *very* weird::
ZopeXMLConfigurationError: File \ "/tmp/endgame/Products/CMFDefault/skin/configure.zcml", line \ 10.2-13.8 ConfigurationError: ('Invalid value for', 'layers', "ImportError:\ Couldn't import default, No module named default in cmf default")
Ah yes. This is due to a missing ZCML declaration in Five. I already fixed it on the Five trunk (http://mail.zope.org/pipermail/checkins/2006-May/001859.html), but the zpt branch is a bit too old to have this fix. That's why it works fine on the Zope trunk but not on the zpt branch.
They I guess we should update the external for Five on the branch, and verify?
No, because Five was branched off too. But all that should be history now, anyways, because I merged everything to the trunk and removed the ZPT branches. Philipp
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Philipp von Weitershausen wrote:
Tres Seaver wrote:
Basically:
* we hardly see a use case for empty path elements. They don't work right now, at least not in the way that you expected them to work. So, we're not really breaking anything if we take away something that didn't work in the first place.
The CMF (and Plone on top of it) work *fine* as they are today for completely empty expressions. Reverting the BBB-incompatible cahnge, and adding deprecation warnings, is our standard practice here.
Yes, for changes that ripped out well-understood and well-supported features. I don't know whether we have a standard practice for cases like this, but there's definitely a straight-forward practice: fix the CMF :).
Not acceptable: the whole purpose of a BBB policy is to allow people sanity when upgrading: *first* you upgrade the lower layers, then you fix the deprecated stuff in the higher layers.
The "empty element" (as opposed to "empty expression", which is what CMF / Plone use) hasn't worked in qute a while, AFAICT. 'unrestrictedTraverse' raises a KeyError in both 2.8 and 2.9.
* AFAIK the CMF tries to compile empty TALES expessions (I don't know yet whether only in tests or also in "real" code, Hanno Schlichting has been testing my branch against the CMF in the past days). This isn't a use case either. It's just misguided usage, as I would call it (Fred agrees). We can fix the CMF to simply not compile empty TALES expressions.
Yes, but you can't break the CMF in a release without doing the deprecation dance first.
Or we could simply fix the CMF. It would have to be done at some point anyways, only that if we do it now it limits the effort. To provide proper BBB for weird behaviour that doesn't even have a good use case is a much larger effort. CMF bugfix releases could be on their way before Zope 2.10 hits even final.
It is still wrong to require people to upgrade a higher-layer because of a needless BBB foul in Zope. We can arrange to do the Right Thing (TM) fairly eaiily: I'm attaching a patch which does that.
- CMFDefault/skin/configure.zcml blows up because the syntax of the 'browser:skin' directive has changed in a BBB-incompatible way. The now-failing directive is::
<browser:skin name="cmf" layers="cmf default" />
and the traceback is *very* weird::
ZopeXMLConfigurationError: File \ "/tmp/endgame/Products/CMFDefault/skin/configure.zcml", line \ 10.2-13.8 ConfigurationError: ('Invalid value for', 'layers', "ImportError:\ Couldn't import default, No module named default in cmf default")
Ah yes. This is due to a missing ZCML declaration in Five. I already fixed it on the Five trunk (http://mail.zope.org/pipermail/checkins/2006-May/001859.html), but the zpt branch is a bit too old to have this fix. That's why it works fine on the Zope trunk but not on the zpt branch.
They I guess we should update the external for Five on the branch, and verify?
No, because Five was branched off too. But all that should be history now, anyways, because I merged everything to the trunk and removed the ZPT branches.
Those tests still break when run against the Zope trunk. Tres. - -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFEfHHe+gerLs4ltQ4RAiKIAKCuRJ8DTmDYDrcPz76JxrAxfc+BOgCfbpYb 4qLOxvsy6OJIDmONqYqbgs4= =le5U -----END PGP SIGNATURE-----
Hi! Tres Seaver wrote:
- CMFDefault/skin/configure.zcml blows up because the syntax of the 'browser:skin' directive has changed in a BBB-incompatible way. The now-failing directive is::
<browser:skin name="cmf" layers="cmf default" />
and the traceback is *very* weird::
ZopeXMLConfigurationError: File \ "/tmp/endgame/Products/CMFDefault/skin/configure.zcml", line \ 10.2-13.8 ConfigurationError: ('Invalid value for', 'layers', "ImportError:\ Couldn't import default, No module named default in cmf default") Ah yes. This is due to a missing ZCML declaration in Five. I already fixed it on the Five trunk (http://mail.zope.org/pipermail/checkins/2006-May/001859.html), but the zpt branch is a bit too old to have this fix. That's why it works fine on the Zope trunk but not on the zpt branch. They I guess we should update the external for Five on the branch, and verify?
No, because Five was branched off too. But all that should be history now, anyways, because I merged everything to the trunk and removed the ZPT branches.
Those tests still break when run against the Zope trunk.
I can't confirm that. I no longer see the failures related to the missing 'default' layer. Most failures were caused by new requirements for the test setup and CMF 2.0 needs a backport of the test fixes on the CMF trunk. So it all boils down to the empty expressions issue and this error: Error in test test_allMetaTypes (Products.CMFCore.tests.test_TypesTool.TypesTool Tests) Traceback (most recent call last): File "/usr/lib/python2.4/unittest.py", line 260, in run testMethod() File ".../cmf21/Products/CMFCore/tests/test_TypesTool.py", line 114, in test_allMetaTypes act = tool.unrestrictedTraverse(html_quote(factype['action'])) File "/usr/local/lib/Zope-2.10/lib/python/OFS/Traversable.py", line 263, in un restrictedTraverse obj = next UnboundLocalError: local variable 'next' referenced before assignment Cheers, Yuppie
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 yuppie wrote:
Hi!
Tres Seaver wrote:
> - CMFDefault/skin/configure.zcml blows up because the syntax of the > 'browser:skin' directive has changed in a BBB-incompatible > way. The > now-failing directive is:: > > <browser:skin > name="cmf" > layers="cmf default" > /> > > and the traceback is *very* weird:: > > ZopeXMLConfigurationError: File \ > "/tmp/endgame/Products/CMFDefault/skin/configure.zcml", line \ > 10.2-13.8 > ConfigurationError: ('Invalid value for', 'layers', > "ImportError:\ > Couldn't import default, No module named default in cmf > default")
Ah yes. This is due to a missing ZCML declaration in Five. I already fixed it on the Five trunk (http://mail.zope.org/pipermail/checkins/2006-May/001859.html), but the zpt branch is a bit too old to have this fix. That's why it works fine on the Zope trunk but not on the zpt branch.
They I guess we should update the external for Five on the branch, and verify?
No, because Five was branched off too. But all that should be history now, anyways, because I merged everything to the trunk and removed the ZPT branches.
Those tests still break when run against the Zope trunk.
I can't confirm that. I no longer see the failures related to the missing 'default' layer.
Most failures were caused by new requirements for the test setup and CMF 2.0 needs a backport of the test fixes on the CMF trunk.
Are these the "no traverser" problems? Those are the ones I said were still broken, and which Philip thought would have been fixed by the newer Five version. CMF 2.0 still exhibilts them, while the CMF trunk does not.
So it all boils down to the empty expressions issue and this error:
I am planning to land my 'empty path expr BBB' patch on the trunk and the 2.10 branch today.
Error in test test_allMetaTypes (Products.CMFCore.tests.test_TypesTool.TypesTool Tests) Traceback (most recent call last): File "/usr/lib/python2.4/unittest.py", line 260, in run testMethod() File ".../cmf21/Products/CMFCore/tests/test_TypesTool.py", line 114, in test_allMetaTypes act = tool.unrestrictedTraverse(html_quote(factype['action'])) File "/usr/local/lib/Zope-2.10/lib/python/OFS/Traversable.py", line 263, in un restrictedTraverse obj = next UnboundLocalError: local variable 'next' referenced before assignment
That's a bug, and a fairly nasty one, introduced in the "traversal refactoring" stuff (revision 67730 of OFS/Traversable.py). The 'name' there is '+', which does *not* get parsed properly by 'nsParse', and which therefore does not bind a 'next' object. The full path being traversed is '+/addFactoryTypeInformation.html'. Lennart, do you have a sense about what it would take to fix that in OFS.Traversable? Tres. - -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFEfIEj+gerLs4ltQ4RAuIsAJ9nhkGS8elceGnq1c22AtsHKYrOLACgwfJh VJ8A4pgV+fRsMd70QFrnNeU= =0Tj/ -----END PGP SIGNATURE-----
Hi! Tres Seaver wrote:
yuppie wrote:
Tres Seaver wrote:
>> - CMFDefault/skin/configure.zcml blows up because the syntax of the >> 'browser:skin' directive has changed in a BBB-incompatible >> way. The >> now-failing directive is:: >> >> <browser:skin >> name="cmf" >> layers="cmf default" >> /> >> >> and the traceback is *very* weird:: >> >> ZopeXMLConfigurationError: File \ >> "/tmp/endgame/Products/CMFDefault/skin/configure.zcml", line \ >> 10.2-13.8 >> ConfigurationError: ('Invalid value for', 'layers', >> "ImportError:\ >> Couldn't import default, No module named default in cmf >> default") > Ah yes. This is due to a missing ZCML declaration in Five. I already > fixed it on the Five trunk > (http://mail.zope.org/pipermail/checkins/2006-May/001859.html), > but the > zpt branch is a bit too old to have this fix. That's why it works > fine > on the Zope trunk but not on the zpt branch. They I guess we should update the external for Five on the branch, and verify?
No, because Five was branched off too. But all that should be history now, anyways, because I merged everything to the trunk and removed the ZPT branches.
Those tests still break when run against the Zope trunk.
I can't confirm that. I no longer see the failures related to the missing 'default' layer.
Most failures were caused by new requirements for the test setup and CMF 2.0 needs a backport of the test fixes on the CMF trunk.
Are these the "no traverser" problems? Those are the ones I said were still broken, and which Philip thought would have been fixed by the newer Five version.
CMF 2.0 still exhibilts them, while the CMF trunk does not.
Yes. The 'no traverser' problems were not caused by the missing 'default' layer, so the Five fix doesn't resolve them. They are caused by the fact that Five traversing is now used in more places, so more tests need setUpTraversing/cleanUp.
So it all boils down to the empty expressions issue and this error:
I am planning to land my 'empty path expr BBB' patch on the trunk and the 2.10 branch today.
I agree with you that there should be BBB code that provides the old behavior and I agree with Philipp that not using that old behavior is a benefit for the CMF. Cheers, Yuppie
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 yuppie wrote:
Yes. The 'no traverser' problems were not caused by the missing 'default' layer, so the Five fix doesn't resolve them. They are caused by the fact that Five traversing is now used in more places, so more tests need setUpTraversing/cleanUp.
OK, thanks for the information.
So it all boils down to the empty expressions issue and this error:
I just created a collector issue for the 'OFS.Traversable' error: http://www.zope.org/Collectors/Zope/2117
I am planning to land my 'empty path expr BBB' patch on the trunk and the 2.10 branch today.
I agree with you that there should be BBB code that provides the old behavior and I agree with Philipp that not using that old behavior is a benefit for the CMF.
Sure. I just don't want to *make* people upgrade CMF when upgrading Zope, unless there is a reason which is important *to the CMF*. An interesting factoid I found while spelunking this issue: the CMF (by way of DCWorkflow) is literally the oldest consumer of the expression machinery outside of ZPT itself! Shane's earliest checkin of the 'Expression' module was nearly 5 years ago, and used an empty string as the class-level default for the 'text' attribute. Tres. - -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFEfJLS+gerLs4ltQ4RAukEAJ92u7KrVU/FVAM6p8dY69GLBtTXWQCgoDzJ 7zp6Sg2I2pckbXThHHvjJdc= =3TF7 -----END PGP SIGNATURE-----
Tres Seaver wrote:
I agree with you that there should be BBB code that provides the old behavior and I agree with Philipp that not using that old behavior is a benefit for the CMF.
Sure. I just don't want to *make* people upgrade CMF when upgrading Zope, unless there is a reason which is important *to the CMF*.
An interesting factoid I found while spelunking this issue: the CMF (by way of DCWorkflow) is literally the oldest consumer of the expression machinery outside of ZPT itself! Shane's earliest checkin of the 'Expression' module was nearly 5 years ago, and used an empty string as the class-level default for the 'text' attribute.
FWIW the following patches gives the proper BBB behaviour. Shall I check it in? Does it need to send a deprecation warning? Florent Index: Products/PageTemplates/Expressions.py =================================================================== --- Products/PageTemplates/Expressions.py (revision 68406) +++ Products/PageTemplates/Expressions.py (working copy) @@ -99,6 +99,8 @@ class ZopePathExpr(PathExpr): def __init__(self, name, expr, engine): + if name == 'standard' and not expr: + expr = 'nothing' super(ZopePathExpr, self).__init__(name, expr, engine, boboAwareZopeTraverse) Florent -- Florent Guillaume, Nuxeo (Paris, France) Director of R&D +33 1 40 33 71 59 http://nuxeo.com fg@nuxeo.com
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Florent Guillaume wrote:
Tres Seaver wrote:
I agree with you that there should be BBB code that provides the old behavior and I agree with Philipp that not using that old behavior is a benefit for the CMF.
Sure. I just don't want to *make* people upgrade CMF when upgrading Zope, unless there is a reason which is important *to the CMF*.
An interesting factoid I found while spelunking this issue: the CMF (by way of DCWorkflow) is literally the oldest consumer of the expression machinery outside of ZPT itself! Shane's earliest checkin of the 'Expression' module was nearly 5 years ago, and used an empty string as the class-level default for the 'text' attribute.
FWIW the following patches gives the proper BBB behaviour. Shall I check it in? Does it need to send a deprecation warning?
Florent
Index: Products/PageTemplates/Expressions.py =================================================================== --- Products/PageTemplates/Expressions.py (revision 68406) +++ Products/PageTemplates/Expressions.py (working copy) @@ -99,6 +99,8 @@ class ZopePathExpr(PathExpr):
def __init__(self, name, expr, engine): + if name == 'standard' and not expr: + expr = 'nothing' super(ZopePathExpr, self).__init__(name, expr, engine, boboAwareZopeTraverse)
I uploaded a similar one, except with deprecation warning, to the collector issue I added: http://www.zope.org/Collectors/Zope/2118 My variation also creates an expression which would raise a KeyError if evaluated, rather than returning None. I don't think anybody can actually be depending on what the current code gives back if an empty expression is called, so yours should be equally fine. WRT dprecation: I would actually prefer no* to have the warning, myself; I don't think that a usage which has been workable for so long is actually in error. We should probably add a test which verifies compilability of empty expressions, plus whichever behavior we specify. Tres. - -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFEfdBg+gerLs4ltQ4RAv3dAJ9nupwaHlRIwjs/0x7jPjswZ8+28ACcDzJj BhQgjD0j0eobvHNhejOo2Mk= =N7uW -----END PGP SIGNATURE-----
On 31 May 2006, at 19:20, Tres Seaver wrote:
I uploaded a similar one, except with deprecation warning, to the collector issue I added:
Ah thanks, I missed it.
My variation also creates an expression which would raise a KeyError if evaluated, rather than returning None. I don't think anybody can actually be depending on what the current code gives back if an empty expression is called, so yours should be equally fine.
If you say it used to return the expression context, then I'm pretty sure nobody used the result. OTOH I'd rather return None just in case, it gives reasonable semantics.
WRT dprecation: I would actually prefer no* to have the warning, myself; I don't think that a usage which has been workable for so long is actually in error.
I agree, we should just decree "in Zope 2, empty ZPT path expression are allowed and evaluate to None". Florent
We should probably add a test which verifies compilability of empty expressions, plus whichever behavior we specify.
-- Florent Guillaume, Nuxeo (Paris, France) Director of R&D +33 1 40 33 71 59 http://nuxeo.com fg@nuxeo.com
On 5/30/06, Tres Seaver <tseaver@palladion.com> wrote:
Lennart, do you have a sense about what it would take to fix that in OFS.Traversable?
Nope, and I have no time until Saturday to check it. -- Lennart Regebro, Nuxeo http://www.nuxeo.com/ CPS Content Management http://www.cps-project.org/
Philipp von Weitershausen wrote at 2006-5-29 18:42 +0200:
...
The CMF (and Plone on top of it) work *fine* as they are today for completely empty expressions. Reverting the BBB-incompatible cahnge, and adding deprecation warnings, is our standard practice here.
Yes, for changes that ripped out well-understood and well-supported features. I don't know whether we have a standard practice for cases like this, but there's definitely a straight-forward practice: fix the CMF :).
But this forces synchronization between the Zope version and the CMF version which then extends to lots of other components build on to of Zope and CMF. Following the usual practice (depracations) allows time to make all components compatible again. Not a too bad approach.... -- Dieter
participants (7)
-
Andreas Jung -
Dieter Maurer -
Florent Guillaume -
Lennart Regebro -
Philipp von Weitershausen -
Tres Seaver -
yuppie