Re: [Zope-dev] Re: exception hiding in _delObject

2004-11-19 Thread Florent Guillaume
In article <[EMAIL PROTECTED]> you write:
> Florent Guillaume wrote:
> 
> > Ok I'll do the change so that the exception is reraised only in debug
> > mode AND if the user is not Manager (so that a Manager can still fix the
> > database).
> 
> Great, thanks very much.

Ok it's on the trunk as r28478.

> > Opinions on whether it can go on the 2.7 branch too ?
> 
> +0.  Andreas is the final arbiter.

Andreas, what do you think ? It's about not swallowing exceptions during
delete when in debug mode and not a Manager. Without this it's very hard
to properly debug indexes and the catalog. The patches don't have any
effect in non-debug mode.

Florent

-- 
Florent Guillaume, Nuxeo (Paris, France)   CTO, Director of R&D
+33 1 40 33 71 59   http://nuxeo.com   [EMAIL PROTECTED]
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 http://mail.zope.org/mailman/listinfo/zope-announce
 http://mail.zope.org/mailman/listinfo/zope )


[Zope-dev] Re: exception hiding in _delObject

2004-11-19 Thread Tres Seaver
Florent Guillaume wrote:
Ok I'll do the change so that the exception is reraised only in debug
mode AND if the user is not Manager (so that a Manager can still fix the
database).
Great, thanks very much.
Opinions on whether it can go on the 2.7 branch too ?
+0.  Andreas is the final arbiter.
Tres.
--
===
Tres Seaver[EMAIL PROTECTED]
Zope Corporation  "Zope Dealers"   http://www.zope.com
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )


Re: [Zope-dev] Re: exception hiding in _delObject

2004-11-19 Thread Florent Guillaume
In article <[EMAIL PROTECTED]> you write:
> Florent Guillaume wrote:
> > In _delObject and manage_beforeDelete there's a try/except that catches 
> > nearly everything (except BeforeDeleteException and the infamous 
> > ConflictError). It then proceeds to log a message, but continues:
> > 
> > try:
> > object.manage_beforeDelete(object, self)
> > except BeforeDeleteException, ob:
> > raise
> > except ConflictError: # Added for CPS
> > raise
> > except:
> > LOG('Zope',ERROR,'manage_beforeDelete() threw',
> > error=sys.exc_info())
> > pass
> > 
> > The rationale is, I guess, that when you delete an object you really 
> > want it gone.
> 
> Yep;  especially as the broken object may be screwing up something 
> *important* to the site.
> 
> > This is IMO very harmful because it hides any bug in the catalog or the 
> > indexes (especially during unit tests where LOG is ignored). I've been 
> > (again) bitten by it.
> > 
> > I'd like to condition the pass to the fact that the current user is 
> > Manager. Otherwise I'd like it to fail (and reraise). So a Manager will 
> > still be able to delete objects when there's a bug, but not others.
> > 
> > Comments ?
> 
> -1.  Buggy application code blocking deletes makes for nightmare "throw 
> away your Data.fs" error scenarios.  I am +0 on it if you can arrange 
> for the new behavior to happen only when Zope is running in debug mode.

Ok I'll do the change so that the exception is reraised only in debug
mode AND if the user is not Manager (so that a Manager can still fix the
database).

Opinions on whether it can go on the 2.7 branch too ?

Florent

-- 
Florent Guillaume, Nuxeo (Paris, France)   CTO, Director of R&D
+33 1 40 33 71 59   http://nuxeo.com   [EMAIL PROTECTED]
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 http://mail.zope.org/mailman/listinfo/zope-announce
 http://mail.zope.org/mailman/listinfo/zope )


Re: [Zope-dev] Re: exception hiding in _delObject

2004-11-10 Thread Dieter Maurer
Tres Seaver wrote at 2004-11-9 13:28 -0500:
>Florent Guillaume wrote:
> ...
>> In _delObject and manage_beforeDelete there's a try/except that catches 
>> nearly everything (except BeforeDeleteException and the infamous 
>> ConflictError). It then proceeds to log a message, but continues:
>> 
>> try:
>> object.manage_beforeDelete(object, self)
>> except BeforeDeleteException, ob:
>> raise
>> except ConflictError: # Added for CPS
>> raise
>> except:
>> LOG('Zope',ERROR,'manage_beforeDelete() threw',
>> error=sys.exc_info())
>> pass
> ...
>> This is IMO very harmful because it hides any bug in the catalog or the 
>> indexes (especially during unit tests where LOG is ignored). I've been 
>> (again) bitten by it.

And in addition, it can leave the ZODB in an inconsistent state.
As you know, "manage_beforeDelete" is a recursive function.
When the exception occurs, it probably had run for part of the
descendants but not for others.
Almost surely, this will give you further unexpected problems
in the future.

>> I'd like to condition the pass to the fact that the current user is 
>> Manager. Otherwise I'd like it to fail (and reraise). So a Manager will 
>> still be able to delete objects when there's a bug, but not others.
>> 
>> Comments ?

I would be even more strict: do not catch the exception at all.

ZODB inconsistencies are as bad when caused by "Manager" intervention
as when caused by "normal" users.

>-1.  Buggy application code blocking deletes makes for nightmare "throw 
>away your Data.fs" error scenarios.  I am +0 on it if you can arrange 
>for the new behavior to happen only when Zope is running in debug mode.

-1 on deleting the object and leaving inconsistent ZODB state
behind. Better fix the application code and then try again to
delete the object.

-- 
Dieter
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 http://mail.zope.org/mailman/listinfo/zope-announce
 http://mail.zope.org/mailman/listinfo/zope )


[Zope-dev] Re: exception hiding in _delObject

2004-11-09 Thread Tres Seaver
Florent Guillaume wrote:
In _delObject and manage_beforeDelete there's a try/except that catches 
nearly everything (except BeforeDeleteException and the infamous 
ConflictError). It then proceeds to log a message, but continues:

try:
object.manage_beforeDelete(object, self)
except BeforeDeleteException, ob:
raise
except ConflictError: # Added for CPS
raise
except:
LOG('Zope',ERROR,'manage_beforeDelete() threw',
error=sys.exc_info())
pass
The rationale is, I guess, that when you delete an object you really 
want it gone.
Yep;  especially as the broken object may be screwing up something 
*important* to the site.

This is IMO very harmful because it hides any bug in the catalog or the 
indexes (especially during unit tests where LOG is ignored). I've been 
(again) bitten by it.

I'd like to condition the pass to the fact that the current user is 
Manager. Otherwise I'd like it to fail (and reraise). So a Manager will 
still be able to delete objects when there's a bug, but not others.

Comments ?
-1.  Buggy application code blocking deletes makes for nightmare "throw 
away your Data.fs" error scenarios.  I am +0 on it if you can arrange 
for the new behavior to happen only when Zope is running in debug mode.

+1 for the ConflictError exception.
Tres.
--
===
Tres Seaver[EMAIL PROTECTED]
Zope Corporation  "Zope Dealers"   http://www.zope.com
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )