Mailing List Archive

On Wed, 2004-10-06 at 10:00, Sidnei da Silva wrote:
> Hello,
>
> I've made some changes on the way ZPublisher deals with
> __bobo_traverse__.
>
> Here's the branch:
> svn+ssh://svn.zope.org/repos/main/Zope/branches/dc-bobo_traverse-branch
>
> Before the changes, exceptions on __bobo_traverse__ would not be
> handled, so if a AttributeError would happen, it would pop up
> untouched, resulting on a 500 response status, instead of 404.

Why is that a good thing? Shouldn't it be up to the implementor to
decide what should happen during bobo traverse? If an exception happens
that raises AttributeError or KeyError during __bt__, I don't think I
want it to try something else.

> It would also try __bobo_traverse__ only, if __bobo_traverse__ exists,
> completely ignoring __getattr__ and __getitem__.


What makes it desirable to use __getattr__ and/or __getitem__ if the
object has a __bobo_traverse__?

> After my change, AttributeError and KeyError in __bobo_traverse__
> will be converted to a NotFound, resulting in a 404 response status.

This seems like a bit more DWIM that would need to be carefully
undocumented, like all the other DWIM. ;-)

> It will also continue looking through __getattr__ and __getitem__,
> which is where the semantics really differ. I'm more worried about
> changing the semantincs because there are other places where
> __bobo_traverse__ is used, namely restrictedTravers on OFS.Traversable
> and PageTemplates.Expressions.

I don't know if this is a win.

>
> This change would simplify the implementations of __bobo_traverse__,
> which would not have to worry about looking at __getattr__ and
> __getitem__ themselves, but I would like to know what people think
> first.

I'm -1, FWIW.

- C


_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Wed, Oct 06, 2004 at 12:29:55PM -0400, Chris McDonough wrote:
| > Before the changes, exceptions on __bobo_traverse__ would not be
| > handled, so if a AttributeError would happen, it would pop up
| > untouched, resulting on a 500 response status, instead of 404.
|
| Why is that a good thing? Shouldn't it be up to the implementor to
| decide what should happen during bobo traverse? If an exception happens
| that raises AttributeError or KeyError during __bt__, I don't think I
| want it to try something else.

I suppose you are referring to the stuff below here. As for converting
into a NotFoundError, Tres suggested to do it in ZPublisher, and
AttributeErrors not coming from __bobo_traverse__ are converted to
NotFound already.

| > It would also try __bobo_traverse__ only, if __bobo_traverse__ exists,
| > completely ignoring __getattr__ and __getitem__.
|
| What makes it desirable to use __getattr__ and/or __getitem__ if the
| object has a __bobo_traverse__?

Simplifying __bobo_traverse__ implementation?

| > After my change, AttributeError and KeyError in __bobo_traverse__
| > will be converted to a NotFound, resulting in a 404 response status.
|
| This seems like a bit more DWIM that would need to be carefully
| undocumented, like all the other DWIM. ;-)

The intention here is not introduce anything new, but to traversal
using __bobo_traverse__ behave the same way as attribute traversal
with regards to exceptions raised.

| > It will also continue looking through __getattr__ and __getitem__,
| > which is where the semantics really differ. I'm more worried about
| > changing the semantincs because there are other places where
| > __bobo_traverse__ is used, namely restrictedTravers on OFS.Traversable
| > and PageTemplates.Expressions.
|
| I don't know if this is a win.

It's probably not, that's why I'm asking :)

| > This change would simplify the implementations of __bobo_traverse__,
| > which would not have to worry about looking at __getattr__ and
| > __getitem__ themselves, but I would like to know what people think
| > first.
|
| I'm -1, FWIW.

Thanks!

--
Sidnei da Silva <sidnei@awkly.org>
http://awkly.org - dreamcatching :: making your dreams come true
http://www.enfoldsystems.com
http://plone.org/about/team#dreamcatcher

<KevinMarks> 'Our series A round is to help us build out the Other Plane; look at the returns possible once we transcend the mortal universe'
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Wed, 2004-10-06 at 12:34, Sidnei da Silva wrote:
> On Wed, Oct 06, 2004 at 12:29:55PM -0400, Chris McDonough wrote:
> | > Before the changes, exceptions on __bobo_traverse__ would not be
> | > handled, so if a AttributeError would happen, it would pop up
> | > untouched, resulting on a 500 response status, instead of 404.
> |
> | Why is that a good thing? Shouldn't it be up to the implementor to
> | decide what should happen during bobo traverse? If an exception happens
> | that raises AttributeError or KeyError during __bt__, I don't think I
> | want it to try something else.
>
> I suppose you are referring to the stuff below here.

It appears I misread this; you don't do getattr or getitem when you get
an AttributeError/KeyError from __bt__, that's good.

FWIW, I'm basically -1 on the publisher turning AttributeErrors into
NotFound errors even though that is the current behavior. I recognize
that the Publisher is itself mostly DWIM but I think this case is a bit
different. I'm not sure whether its more wrong to be inconsistent or
over-implicit wrt differences between __geitem__/_getattr__ and __bt__,
but I think I'll choose inconsistent today.

> As for converting
> into a NotFoundError, Tres suggested to do it in ZPublisher, and
> AttributeErrors not coming from __bobo_traverse__ are converted to
> NotFound already.

Yup.

> | > It would also try __bobo_traverse__ only, if __bobo_traverse__ exists,
> | > completely ignoring __getattr__ and __getitem__.
> |
> | What makes it desirable to use __getattr__ and/or __getitem__ if the
> | object has a __bobo_traverse__?
>
> Simplifying __bobo_traverse__ implementation?

FWIW, I really haven't written many __bobo_traverse__ implementations,
but when I have, I haven't thought too much about how hard it was or
that it could be simpler. I'm actually glad the framework doesn't try
too hard to help, it would have really screwed me up in a few cases
(AttributeError and KeyError seem to be pretty common).

> | > After my change, AttributeError and KeyError in __bobo_traverse__
> | > will be converted to a NotFound, resulting in a 404 response status.
> |
> | This seems like a bit more DWIM that would need to be carefully
> | undocumented, like all the other DWIM. ;-)
>
> The intention here is not introduce anything new, but to traversal
> using __bobo_traverse__ behave the same way as attribute traversal
> with regards to exceptions raised.

I guess the difference is that the publisher is trying to catch
AttreibuteError/KeyError/TypeError/IndexError as a result of its own
operations to *get* the object. I don't think it was ever intended that
if you raise an AttributeError/KeyError from app code (implemented, in
this case, in __bt__) that it would have the side effect of causing a
NotFoundError. I know that's the effect of the current code, but I
really don't think that when the Publisher was implemented that this was
ever considered.

> | > It will also continue looking through __getattr__ and __getitem__,
> | > which is where the semantics really differ. I'm more worried about
> | > changing the semantincs because there are other places where
> | > __bobo_traverse__ is used, namely restrictedTravers on OFS.Traversable
> | > and PageTemplates.Expressions.
> |
> | I don't know if this is a win.
>
> It's probably not, that's why I'm asking :)

Sure...

- C


_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Wed, Oct 06, 2004 at 12:54:14PM -0400, Chris McDonough wrote:
| > I suppose you are referring to the stuff below here.
|
| It appears I misread this; you don't do getattr or getitem when you get
| an AttributeError/KeyError from __bt__, that's good.

I actually do right now, but I feel it's wrong. ZPublisher didn't do
it before. If you say not doing it is good, then I'm surely removing
it.

| > The intention here is not introduce anything new, but to traversal
| > using __bobo_traverse__ behave the same way as attribute traversal
| > with regards to exceptions raised.
|
| I guess the difference is that the publisher is trying to catch
| AttreibuteError/KeyError/TypeError/IndexError as a result of its own
| operations to *get* the object. I don't think it was ever intended that
| if you raise an AttributeError/KeyError from app code (implemented, in
| this case, in __bt__) that it would have the side effect of causing a
| NotFoundError. I know that's the effect of the current code, but I
| really don't think that when the Publisher was implemented that this was
| ever considered.

The main reasoning for all of this was that:

- __bt__ exceptions would pass by unmodified on ZPublisher
- so we thought raising NotFound from __bt__ would be fine
- but then it broke PageTemplates.Expressions (SubPathExpression)
because it doesn't expect a NotFound
- raising AttributeError copes well with PageTemplates.Expressions
- but in the case of ZPublisher it should really be NotFound, as it is
really coming from the browser, and not trying to find an attribute
in application code
- Tres said that NotFound should not be raised by application code,
but only by ZPublisher.

So, just to clarify, are you ok with converting the
AttributeError/KeyError to NotFound on ZPublisher and not trying to
fallback to __getattr__/__getitem__?


--
Sidnei da Silva <sidnei@awkly.org>
http://awkly.org - dreamcatching :: making your dreams come true
http://www.enfoldsystems.com
http://plone.org/about/team#dreamcatcher

I bet the human brain is a kludge.
-- Marvin Minsky
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Wed, 2004-10-06 at 13:06, Sidnei da Silva wrote:
> On Wed, Oct 06, 2004 at 12:54:14PM -0400, Chris McDonough wrote:
> | > I suppose you are referring to the stuff below here.
> |
> | It appears I misread this; you don't do getattr or getitem when you get
> | an AttributeError/KeyError from __bt__, that's good.
>
> I actually do right now, but I feel it's wrong. ZPublisher didn't do
> it before. If you say not doing it is good, then I'm surely removing
> it.
>
> | > The intention here is not introduce anything new, but to traversal
> | > using __bobo_traverse__ behave the same way as attribute traversal
> | > with regards to exceptions raised.
> |
> | I guess the difference is that the publisher is trying to catch
> | AttreibuteError/KeyError/TypeError/IndexError as a result of its own
> | operations to *get* the object. I don't think it was ever intended that
> | if you raise an AttributeError/KeyError from app code (implemented, in
> | this case, in __bt__) that it would have the side effect of causing a
> | NotFoundError. I know that's the effect of the current code, but I
> | really don't think that when the Publisher was implemented that this was
> | ever considered.
>
> The main reasoning for all of this was that:
>
> - __bt__ exceptions would pass by unmodified on ZPublisher
> - so we thought raising NotFound from __bt__ would be fine
> - but then it broke PageTemplates.Expressions (SubPathExpression)
> because it doesn't expect a NotFound

Yup got it so far.

> - raising AttributeError copes well with PageTemplates.Expressions
> - but in the case of ZPublisher it should really be NotFound, as it is
> really coming from the browser, and not trying to find an attribute
> in application code

But isn't __bt__ "application code"? Maybe not. Maybe that's what I'm
stuck on.

IMO, we should really have a sentinel exception to indicate something
that should be raised up to the publisher and which should cause a not
found error (something with the same "punch up through the stack"
semantics as ConflictError, IOW). But I realize we don't have that now
for __getitem__/__getattr__ and things still appear to work ok, so maybe
I'm just being dogmatic about it.

> - Tres said that NotFound should not be raised by application code,
> but only by ZPublisher.

Agreed. Although, maybe in the case of __bt__, you could bend this
rule? I really don't know what the right answer is.

> So, just to clarify, are you ok with converting the
> AttributeError/KeyError to NotFound on ZPublisher and not trying to
> fallback to __getattr__/__getitem__?

I guess I'd be -0 on it given this explanation of the problem you're
trying to solve.

- C


_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Wed, Oct 06, 2004 at 01:30:40PM -0400, Chris McDonough wrote:
| > - Tres said that NotFound should not be raised by application code,
| > but only by ZPublisher.
|
| Agreed. Although, maybe in the case of __bt__, you could bend this
| rule? I really don't know what the right answer is.

I could, but then it breaks PageTemplates path expressions. :(

| > So, just to clarify, are you ok with converting the
| > AttributeError/KeyError to NotFound on ZPublisher and not trying to
| > fallback to __getattr__/__getitem__?
|
| I guess I'd be -0 on it given this explanation of the problem you're
| trying to solve.

That's good enough for me :)

--
Sidnei da Silva <sidnei@awkly.org>
http://awkly.org - dreamcatching :: making your dreams come true
http://www.enfoldsystems.com
http://plone.org/about/team#dreamcatcher

<allexpro> discovering twisted is probably the best thing that has happened in my life
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Wed, 2004-10-06 at 13:36, Sidnei da Silva wrote:
> On Wed, Oct 06, 2004 at 01:30:40PM -0400, Chris McDonough wrote:
> | > - Tres said that NotFound should not be raised by application code,
> | > but only by ZPublisher.
> |
> | Agreed. Although, maybe in the case of __bt__, you could bend this
> | rule? I really don't know what the right answer is.
>
> I could, but then it breaks PageTemplates path expressions. :(

Yup! ;-)

> | > So, just to clarify, are you ok with converting the
> | > AttributeError/KeyError to NotFound on ZPublisher and not trying to
> | > fallback to __getattr__/__getitem__?
> |
> | I guess I'd be -0 on it given this explanation of the problem you're
> | trying to solve.
>
> That's good enough for me :)

OK, but I reserve the right to send the PSU after you when it hides one
of my application errors. ;-)

- C


_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Wed, Oct 06, 2004 at 01:42:49PM -0400, Chris McDonough wrote:
| OK, but I reserve the right to send the PSU after you when it hides one
| of my application errors. ;-)

Fair enough!

--
Sidnei da Silva <sidnei@awkly.org>
http://awkly.org - dreamcatching :: making your dreams come true
http://www.enfoldsystems.com
http://plone.org/about/team#dreamcatcher

Deliver yesterday, code today, think tomorrow.
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

--On Mittwoch, 6. Oktober 2004 14:43 Uhr -0300 Sidnei da Silva
<sidnei@awkly.org> wrote:

> On Wed, Oct 06, 2004 at 01:42:49PM -0400, Chris McDonough wrote:
>| OK, but I reserve the right to send the PSU after you when it hides one
>| of my application errors. ;-)
>
PSU?

-aj
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Wed, 2004-10-06 at 13:47, Andreas Jung wrote:
> --On Mittwoch, 6. Oktober 2004 14:43 Uhr -0300 Sidnei da Silva
> <sidnei@awkly.org> wrote:
>
> > On Wed, Oct 06, 2004 at 01:42:49PM -0400, Chris McDonough wrote:
> >| OK, but I reserve the right to send the PSU after you when it hides one
> >| of my application errors. ;-)
> >
> PSU?

<http://www.zopezen.org/Members/zopista/News_Item.2003-10-20.0634/talkback/1066765183>


_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

Chris McDonough wrote:
> On Wed, 2004-10-06 at 13:47, Andreas Jung wrote:
>
>>--On Mittwoch, 6. Oktober 2004 14:43 Uhr -0300 Sidnei da Silva
>><sidnei@awkly.org> wrote:
>>
>>
>>>On Wed, Oct 06, 2004 at 01:42:49PM -0400, Chris McDonough wrote:
>>>| OK, but I reserve the right to send the PSU after you when it hides one
>>>| of my application errors. ;-)
>>>
>>
>>PSU?
>
>
> <http://www.zopezen.org/Members/zopista/News_Item.2003-10-20.0634/talkback/1066765183>
>

Ignore that, nothing to see here. The Python Secret Underground doesn't
exist.

But, if it did exist, it would be an organisation which dominated the
internet completely enabling them to
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

> Ignore that, nothing to see here. The Python Secret Underground
> doesn't exist.
>
> But, if it did exist, it would be an organisation which dominated the
> internet completely enabling them to
> _______________________________________________
> Zope-Coders mailing list
> Zope-Coders@zope.org
> http://mail.zope.org/mailman/listinfo/zope-coders
>


Yikes, they managed to grab him before he could even finish his email!

_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Wed, Oct 06, 2004 at 09:28:41PM +0200, Jens Vagelpohl wrote:
> >Ignore that, nothing to see here. The Python Secret Underground
> >doesn't exist.
> >
> >But, if it did exist, it would be an organisation which dominated the
> >internet completely enabling them to
> >_______________________________________________
> >Zope-Coders mailing list
> >Zope-Coders@zope.org
> >http://mail.zope.org/mailman/listinfo/zope-coders
> >
>
>
> Yikes, they managed to grab him before he could even finish his email!

They do that all the time, those evil bast

_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

G'day,

thought I'd throw in my 2c as a person who has written/dealt with quite
a few __bobo__traverse methods inside applications.

On Thu, 2004-10-07 at 00:00, Sidnei da Silva wrote:
> Hello,
>
> I've made some changes on the way ZPublisher deals with
> __bobo_traverse__.
>
> Here's the branch:
> svn+ssh://svn.zope.org/repos/main/Zope/branches/dc-bobo_traverse-branch
>
> Before the changes, exceptions on __bobo_traverse__ would not be
> handled, so if a AttributeError would happen, it would pop up
> untouched, resulting on a 500 response status, instead of 404.
>
> It would also try __bobo_traverse__ only, if __bobo_traverse__ exists,
> completely ignoring __getattr__ and __getitem__.
>
> After my change, AttributeError and KeyError in __bobo_traverse__
> will be converted to a NotFound, resulting in a 404 response status.

This seems fine to me at first glance. A colleague was concerned that
KeyError more often means you have screwed up your application code, so
converting them to a 404 might mask things.

> It will also continue looking through __getattr__ and __getitem__,
> which is where the semantics really differ. I'm more worried about
> changing the semantincs because there are other places where
> __bobo_traverse__ is used, namely restrictedTravers on OFS.Traversable
> and PageTemplates.Expressions.
>
> This change would simplify the implementations of __bobo_traverse__,
> which would not have to worry about looking at __getattr__ and
> __getitem__ themselves, but I would like to know what people think
> first.

I think what you are suggesting is falling back to __getattr__ and
__getitem__ after __bobo_traverse__ raises a KeyError or AttributeError,
yeah? Or is that only on returning None?

I don't like this. At the application level we write __bobo_traverse__
methods to override how it behaves. This means we don't want it falling
back to default behaviour when our __bobo_traverse__ method says "no,
you can't traverse there". Possibly even worse is falling back to
default behaviour when our application code is buggy and raises
unexpected exceptions.

Thinking about it some more, I guess I'm -1.

Exceptions raised in __bobo_traverse__ currently mean, and usually
indicate, a bug in your application code. If you really want a 404
response, you do so by returning None. Changing things so that
AttributeError _also_ means 404, or even worse, means fall back to
default __getattr__ behaviour, will confuse things.

--
Donovan Baarda <abo@minkirri.apana.org.au>
http://minkirri.apana.org.au/~abo/

_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Thu, Oct 07, 2004 at 11:10:05AM +1000, Donovan Baarda wrote:
Returning None gives a 'object cannot be published because doesn't
have a docstring.' which is even worse.

--
Sidnei da Silva <sidnei@awkly.org>
http://awkly.org - dreamcatching :: making your dreams come true
http://www.enfoldsystems.com
http://plone.org/about/team#dreamcatcher

The program isn't debugged until the last user is dead.
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Thu, 2004-10-07 at 11:13, Sidnei da Silva wrote:
> On Thu, Oct 07, 2004 at 11:10:05AM +1000, Donovan Baarda wrote:
> Returning None gives a 'object cannot be published because doesn't
> have a docstring.' which is even worse.

Hmm... didn't notice that... nasty.

So what is the current way for a __bobo_traverse__ to indicate "object
does not exist" and result in a 404?

Perhaps AttributeError = 404 is a good idea after all...

--
Donovan Baarda <abo@minkirri.apana.org.au>
http://minkirri.apana.org.au/~abo/

_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Thu, 7 Oct 2004 11:19 am, Donovan Baarda wrote:
> On Thu, 2004-10-07 at 11:13, Sidnei da Silva wrote:
> > On Thu, Oct 07, 2004 at 11:10:05AM +1000, Donovan Baarda wrote:
> > Returning None gives a 'object cannot be published because doesn't
> > have a docstring.' which is even worse.
>
> Hmm... didn't notice that... nasty.
>
> So what is the current way for a __bobo_traverse__ to indicate "object
> does not exist" and result in a 404?
>
> Perhaps AttributeError = 404 is a good idea after all...

No, as already mentioned, this will mask any application bugs that raise
AttributeError.

NotFound exists for this purpose, doesn't it?


Richard
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFBZJqsrGisBEHG6TARAl3LAJ93Dz9ifztxbrs6A4tFMYvwfAUrCwCfejbb
TYbvGBvxllomSNb3tngU5LI=
=kIw7
-----END PGP SIGNATURE-----
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

| Hmm... didn't notice that... nasty.
|
| So what is the current way for a __bobo_traverse__ to indicate "object
| does not exist" and result in a 404?

There's no way :) You could raise NotFound yourself, but then it
breaks PageTemplates.

| Perhaps AttributeError = 404 is a good idea after all...

It's the best I could come up with.

--
Sidnei da Silva <sidnei@awkly.org>
http://awkly.org - dreamcatching :: making your dreams come true
http://www.enfoldsystems.com
http://plone.org/about/team#dreamcatcher

<Tazman> damn my office is cold.
<Tazman> need a hot secretary to warm it up.
-- Seen on #Linux
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Thu, 2004-10-07 at 11:24, Sidnei da Silva wrote:
> | Hmm... didn't notice that... nasty.
> |
> | So what is the current way for a __bobo_traverse__ to indicate "object
> | does not exist" and result in a 404?
>
> There's no way :) You could raise NotFound yourself, but then it
> breaks PageTemplates.
>
> | Perhaps AttributeError = 404 is a good idea after all...
>
> It's the best I could come up with.

It seems there are several possible solutions to this;

1) Fix PageTemplates so that NotFound doesn't break it (dunno what is
involved).

2) Fix returning None so that it returns 404 instead of "No Docstring",
and introduce some exception for the "No Docstring" case.

3) make AttributeError = 404, and document it like hell so people know
to catch them and raise as some other Error if they don't want a 404.

To me 1) seems like a good idea regardless. For 2) I really don't
understand why None results in "No Docstring"... is this really right?
If NotFound worked, I'm not sure what returning None should mean, but
"No Docstring" doesn't seem to be it. Option 3) seems unattractive to
me.

--
Donovan Baarda <abo@minkirri.apana.org.au>
http://minkirri.apana.org.au/~abo/

_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

Richard Jones wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Thu, 7 Oct 2004 11:19 am, Donovan Baarda wrote:
>
>>On Thu, 2004-10-07 at 11:13, Sidnei da Silva wrote:
>>
>>>On Thu, Oct 07, 2004 at 11:10:05AM +1000, Donovan Baarda wrote:
>>>Returning None gives a 'object cannot be published because doesn't
>>>have a docstring.' which is even worse.
>>
>>Hmm... didn't notice that... nasty.
>>
>>So what is the current way for a __bobo_traverse__ to indicate "object
>>does not exist" and result in a 404?
>>
>>Perhaps AttributeError = 404 is a good idea after all...
>
>
> No, as already mentioned, this will mask any application bugs that raise
> AttributeError.
>
> NotFound exists for this purpose, doesn't it?

NotFound is an HTTP Protocol level mapping for an error; it should
*not* be raised by application code. In particular, __bobo_traverse__
is called *both* by the publisher *and* by unrestrictedTraverse;
raising NotFound is *totally* unexpected and inappropriate for the
second case. The current schizophrenia among its callers makes writing
a sane __bobo_traverse__ implementation logically impossible.

BTW, note that the publisher *already* DWIMs AttributeError and KeyError
into NotFound for traversals which do not have a __bobo_traverse__;
the proposed change just makes the DWIM more regular, and thus more
predictable.

The alternatives are:

- As Sidnei proposed, have the publisher map {Attribute,Key}Errors
raised by __bobo_traverse__ onto NotFound, except when running in
debug mode. Don't modify unrestrictedTraverse at all.

- Define a special error, e.g. OFS.Traversable.Nonesuch, and make
raising it part of the contract for __bobo_traverse__; raising
{Attribute,Key,Index,Value}Error from __bobo_traverse__ would then
be an unambiguous application error. We should still decide what to
do about such errors, and should probably remove the publisher's
current DWIM (when it calls getattr / getitem directly). The
downside here is having to modify the dozen or so __bobo_traverse__
implementations in the core, plus breaking uncountable third-party
implementations, for the sake of API purity.

- Morph NotFound into a more generic exception, and make it part of
the contract (note that this is essentially identical to the
previous alternative, except for spelling).

- Make returning 'None' for "not found" the contract for
__bobo_traverse__, and modify both the publisher and
unrestrictedTraverse to look for it. Again, we would need to
revisit the core implementations, and risk breaking third-party
versions.

- Define something like Zope3's exception views, and remove all the
hardwired exception DWIM from the publisher (not feasible, but one
of the logical alternatives).

- Continue to have the current set of inconsistent mappings of
{Attribute,Key}Error to NotFound in some cases, but propagated
as ServerError in others (note that the DWIM in question is already
suppressed when running in debug mode, where losing the original
exception's information might actually be a problem).

Tres.
--
===============================================================
Tres Seaver tseaver@zope.com
Zope Corporation "Zope Dealers" http://www.zope.com

_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

Donovan Baarda wrote:
> On Thu, 2004-10-07 at 11:24, Sidnei da Silva wrote:
>
>>| Hmm... didn't notice that... nasty.
>>|
>>| So what is the current way for a __bobo_traverse__ to indicate "object
>>| does not exist" and result in a 404?
>>
>>There's no way :) You could raise NotFound yourself, but then it
>>breaks PageTemplates.
>>
>>| Perhaps AttributeError = 404 is a good idea after all...
>>
>>It's the best I could come up with.
>
>
> It seems there are several possible solutions to this;
>
> 1) Fix PageTemplates so that NotFound doesn't break it (dunno what is
> involved).
>
> 2) Fix returning None so that it returns 404 instead of "No Docstring",
> and introduce some exception for the "No Docstring" case.
>
> 3) make AttributeError = 404, and document it like hell so people know
> to catch them and raise as some other Error if they don't want a 404.
>
> To me 1) seems like a good idea regardless. For 2) I really don't
> understand why None results in "No Docstring"... is this really right?
> If NotFound worked, I'm not sure what returning None should mean, but
> "No Docstring" doesn't seem to be it. Option 3) seems unattractive to
> me.

The publisher's job has always been to mediate between what the protocl
(HTTP) expects, and what the application delivers. In this case, the
difference is between having the HTTP response code be 404, or having it
be 500. Making either choice is DWIM ("do what I mean"); it happens to
be consistent with the other publisher code (immediately following the
test for __bobo_traverse__) to treat it as a 404 instead of a 500.

In debug mode, we should prefer a 500, because it preserves the original
exception information; this is the current behavior for errors raised
from getattr / getitem, and would be the behavior under Sidnei's
proposed change.

Tres.
--
===============================================================
Tres Seaver tseaver@zope.com
Zope Corporation "Zope Dealers" http://www.zope.com

_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

On Thu, 2004-10-07 at 12:11, Tres Seaver wrote:
> Richard Jones wrote:
[...]
> NotFound is an HTTP Protocol level mapping for an error; it should
> *not* be raised by application code. In particular, __bobo_traverse__
> is called *both* by the publisher *and* by unrestrictedTraverse;
> raising NotFound is *totally* unexpected and inappropriate for the
> second case. The current schizophrenia among its callers makes writing
> a sane __bobo_traverse__ implementation logically impossible.
[...]

Hmmm. so much for NotFound... explains why I've never seen it used by
applications.

> The alternatives are:
>
> - As Sidnei proposed, have the publisher map {Attribute,Key}Errors
> raised by __bobo_traverse__ onto NotFound, except when running in
> debug mode. Don't modify unrestrictedTraverse at all.

The fact that this is off in debug mode is good... I'm slowly being
converted into a +1 for AttributeError = 404.

I'm definitely still -1 for AttributeError in __bobo_traverse__ means
fallback to __getattr__, which was the second part of the branch.

> - Define a special error, e.g. OFS.Traversable.Nonesuch, and make
> raising it part of the contract for __bobo_traverse__; raising
> {Attribute,Key,Index,Value}Error from __bobo_traverse__ would then
> be an unambiguous application error. We should still decide what to
> do about such errors, and should probably remove the publisher's
> current DWIM (when it calls getattr / getitem directly). The
> downside here is having to modify the dozen or so __bobo_traverse__
> implementations in the core, plus breaking uncountable third-party
> implementations, for the sake of API purity.

too big a change. -1.

> - Morph NotFound into a more generic exception, and make it part of
> the contract (note that this is essentially identical to the
> previous alternative, except for spelling).

Not sure... I think I agree that NotFound is for the publisher only, not
applications.

> - Make returning 'None' for "not found" the contract for
> __bobo_traverse__, and modify both the publisher and
> unrestrictedTraverse to look for it. Again, we would need to
> revisit the core implementations, and risk breaking third-party
> versions.
[...]

Does anyone currently really use and expect returning None to mean "no
docstring"?


--
Donovan Baarda <abo@minkirri.apana.org.au>
http://minkirri.apana.org.au/~abo/

_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

Donovan Baarda wrote:
> On Thu, 2004-10-07 at 12:11, Tres Seaver wrote:
>
>>The alternatives are:
>>
>> - As Sidnei proposed, have the publisher map {Attribute,Key}Errors
>> raised by __bobo_traverse__ onto NotFound, except when running in
>> debug mode. Don't modify unrestrictedTraverse at all.
>
>
> The fact that this is off in debug mode is good... I'm slowly being
> converted into a +1 for AttributeError = 404.
>
> I'm definitely still -1 for AttributeError in __bobo_traverse__ means
> fallback to __getattr__, which was the second part of the branch.

Right; I think Sidnei has already reverted that part.

> Does anyone currently really use and expect returning None to mean "no
> docstring"?

The "no docstring" is because the "published" object is None. The
publisher refuses to render any published object which has no docstring,
due to a very early piece of bobo DWIM: it was an attempt to
distinguish between methods which were part of the object's public API
vs. its private implementation.

Tres.
--
===============================================================
Tres Seaver tseaver@zope.com
Zope Corporation "Zope Dealers" http://www.zope.com
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

In article <20041006140019.GC4591@cotia.awkly.org> you write:
> I've made some changes on the way ZPublisher deals with
> __bobo_traverse__.
>
> Here's the branch:
> svn+ssh://svn.zope.org/repos/main/Zope/branches/dc-bobo_traverse-branch
>
[...]
> This change would simplify the implementations of __bobo_traverse__,
> which would not have to worry about looking at __getattr__ and
> __getitem__ themselves, but I would like to know what people think
> first.
>
> The changes would be applied to 2_7-branch and trunk.

I'm all for these cleanups, or a variation on them, but I'm strongly -1
on them being included in the 2.7 branch. 2.7 is getting very stable
now, and I don't want any kind of incompatibility problems added.

Florent

--
Florent Guillaume, Nuxeo (Paris, France)
+33 1 40 33 71 59 http://nuxeo.com mailto:fg@nuxeo.com
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders