Florent Guillaume wrote:
<Sigh> Please read and understand the code more carefully.

There is no expected error. reindexObjectSecurity (which I introduced
more than 2 years ago because CMF was buggy without it) needs access to
all subobjects to reindex them. To do that it has to:

1. use an unrestricted searchResults, which was introduced with the name
   unrestrictedSearchResults 8 months ago in the types tool, and used in
   reindexObjectSecurity 3 weeks ago,

2. use an unrestricted equivalent of getObject. Before, getObject
   returned the object without security checks. Now, it does security
   checks. So we can't use it. And there's no unrestrictedGetObject.

That's why we have to do the equivalent using unrestrictedTraverse and
brain.getPath()

Okay, all this I agree with.

The None issue is a red herring, it's just that we *don't* want to fail
even if there are broken indexes. No we don't. We're reindexing what we
can find in the catalog. We don't want an exception.

Yes we do! I really really really want to know if:
- I have a catalog entry that points to an object that no longer exists. This can ONLY happen due to a bug somewhere and needs to be fixed
- I have index corruption or other weirdness
- any of the above are happening, and be able to descriminate between them.


I honestly can thing of no sane justification for putting that None there. It's tantamount to a bare try except, and without even any logging.

PLEASE provide some sane justification for this, 'cos I'm obviously missing something since you're a bright guy and can't seriously be advocating sweeping unspecified bugs under the carpet...

I don't want the users to find that out when they change the local roles
somewhere.

When DO you want them to find out?

If you want to audit your catalog, just write a recursive
method that checks the objects.

Why should anyone have to do an explicit audit just to find errors that are being reported but hidden by day to day use of the catalog?


What you're saying is like "yes, I want to catch and ignore conflict errors when a user saves a document, 'cos you should go and check for resulting data inconsistencies in an offline process"...

Don't do it on the fly and have it fail
on your users when ignoring broken object would have no actual impact on
the functionality of the code.

It has impact on the funcionality of the system as a whole and gets worse the longer you leave it...


Now, this particular case seems to back up my point even more. It looks like the actual problem here is crufty code in Products.CMFDefault.DiscussionItem which has been happilly masked up until now.

This is unrelated. Don't mix theses issues, I fixed a real bug that may or may not have been visible because of DiscussionItem.

How so? Sidnei would never have seen the traceback if you had swallowed the problem by replacing getObject with unrestrictedTraverse(path,None), he would have had something else weird happen later down the line and have to try and track it back to this action going wrong...


Really, what am I missing here?

cheers,

Chris

--
Simplistix - Content Management, Zope & Python Consulting
           - http://www.simplistix.co.uk
_______________________________________________
Zope-CMF maillist  -  Zope-CMF@lists.zope.org
http://mail.zope.org/mailman/listinfo/zope-cmf

See http://collector.zope.org/CMF for bug reports and feature requests

Reply via email to