Sorry for the delay, very busy pas two weeks here.



I think we can mark this as "Not a bug" because I've already made 
CyrSQL not extend from SQL driver. Will upload it to the new ticket 
shortly, after someone will answer my question about it in the dev@ 
list.

Ping again?

Ping?

You would have to refactor the removeUser() methods. Either split off 
the actual user removal into a separate method, say _removeUser() and 
call that from cyrsql instead of parent::removeUser(), or add an 
additional parameter to Auth_sql::removeUser() to *not* run 
removeUserData(), and set this parameter when called from Auth_cyrsql. 
The first one is cleaner IMO.

Ah, OK. What could be a possible compromise here?

Yes, but it no longer is passed through Auth_sql::removeUser() to the 
admin interface calling removeUser(). Only if using the cyrsql driver, 
but not if using the sql driver.

Yes.
Ghm, correct me if I'm wrong but isn't application's removeUserData() 
call reports these errors anyway? With my patch and *without* 
http://bugs.horde.org/ticket/6968 patches I'm still getting errors 
from nag, mnemo etc. BUT the user is removed succefully from the 
backend. What is the difference between calling removeUser() from 
Auth_cyrsql and Auth_sql? As I see in the source it executes exactly 
the same API call in the needed applications.

As far as I understand from Chuck's comment, you only need this 
because you are using the cyrsql driver and don't want to stop 
removing the user if parent::removeUser() is returning an error due to 
the removeUserData() call in the sql driver. Is this correct?



But you patch unconditionally removes returning errors from 
Auth_sql::removeUser() so this won't report any errors for people 
using the sql driver directly. This is of course not acceptable.

Sorry, I don't understand. What exactly do you mean?

How, if the removeUserData() implementations don't return an error?

Well, PEAR::Error is returned in applications's removeUserData anyway 
except that in notification message it's not horde.warning, it's 
horde.error.

We should not stop the removal process if removeUserData fails, but 
I'm with Chuck here, we should still raise a warning message, and then 
continue.

Yep, it's Cyrus.



The question is: do we consider user data as _vital_ part of user 
removal process? I don't see why it should be.

Oh, this is just for the SQL driver - I misunderstood. What sort of 
dependencies are you talking about, though - is this cyrus again?

No, ignoring any errors isn't the way to go here - instead, if the 
process should _really_ continue, the calling code should log the 
result and then continue.

If removeUser() is not able to remove user data, it should not return 
false, because drivers that depend on it stops further removal of a 
user from other backends.