[Freeipa-devel] [PATCH] 635 wait for memberof plugin when doing reverse members

[Jan Zeleny]

Rob Crittenden <rcritten at redhat.com> wrote:
> Jan Zelený wrote:
> > Rob Crittenden<rcritten at redhat.com>  wrote:
> >> Give the memberof plugin time to work when adding/removing reverse
> >> members.
> >> 
> >> When we add/remove reverse members it looks like we're operating on
> >> group A but we're really operating on group B. This adds/removes the
> >> member attribute on group B and the memberof plugin adds the memberof
> >> attribute into group A.
> >> 
> >> We need to give the memberof plugin a chance to do its work so loop a
> >> few times, reading the entry to see if the number of memberof is more or
> >> less what we expect. Bail out if it is taking too long.
> >> 
> >> ticket 560
> >> 
> >> rob
> > 
> > About that FIXME you got there: I'm not sure if it wouldn't be better to
> > handle the possible exception right in the wait_for_memberof method (I
> > guess it depends on what exception are we expecting and what are we
> > going to do with it?). If you want the exception to reach the calling
> > function, I'd like to see some kind of exception handling in that
> > function - either to let the user know that the error occurred during
> > this waiting or maybe to disregard the exception and continue normal
> > operation.
> 
> The types of exceptions could run the gambit but I was wondering what
> would happen if we were looping and some other user deleted the role.
> The next search for it would fail with NotFound. Granted this isn't a
> very friendly message to return to someone after adding a member to the
> group but it does sort of make sense (someone deleted it concurrently).
> It seemed best to just let this filter up to the caller.

Yes, I understand that. But my point was that it would be more user friendy to 
catch this exception in the calling function and adjust the error message to 
the situation. Otherwise user can get completely out-of-context error message, 
like "user not found" when working with groups or something like that.

> 
> > Some nitpicking: I'm confused - in the doc string you state that "this
> > will loop for 6+ seconds" and a couple lines below, you have a comment
> > "Don't sleep for more that 6 seconds" - is there a mistake ar are these
> > two statements unrelated?
> 
> Yeah, I was afraid that might be confusing. I'll wait .3 seconds 20
> times so 6 seconds. There are a few LDAP calls which take a bit of time
> as well, so it will be 6+ seconds if it goes the whole time.

Ok, thanks for explanation

Jan