Re: [Freeipa-devel] [PATCH] 635 wait for memberof plugin when doing reverse members
Jan Zelený wrote: Rob Crittenden wrote: Jan Zeleny wrote: Rob Crittenden wrote: Jan Zelený wrote: Rob Crittendenwrote: 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 Ok, I added a catch-all in case something goes horribly wrong. rob ack Jan pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 635 wait for memberof plugin when doing reverse members
Jan Zeleny wrote:
Rob Crittenden wrote:
Jan Zelený wrote:
Rob Crittenden 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
Ok, I added a catch-all in case something goes horribly wrong.
rob
>From 59e21c840e096ebf693bc9cdcc93c939b06cfd2c Mon Sep 17 00:00:00 2001
From: Rob Crittenden
Date: Mon, 13 Dec 2010 11:10:37 -0500
Subject: [PATCH] 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
---
ipalib/errors.py | 17 +++
ipalib/plugins/baseldap.py | 65 +--
2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/ipalib/errors.py b/ipalib/errors.py
index fe2b01e..d9bf385 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1157,6 +1157,23 @@ class ManagedGroupExistsError(ExecutionError):
errno = 4024
format = _('Unable to create private group. Group \'%(group)s\' already exists.')
+
+class ReverseMemberError(ExecutionError):
+"""
+**4025** Raised when verifying that all reverse members have been added or removed.
+
+For example:
+
+>>> raise ReverseMemberError(verb=u'added', exc=u'Group \'foo\' not found.')
+Traceback (most recent call last):
+ ...
+A problem was encounted when verifying that all members were added: Group 'foo' not found.
+"""
+
+errno = 4025
+format = _('A problem was encounted when verifying that all members were %(verb)s: %(exc)s')
+
+
class BuiltinError(ExecutionError):
"""
**4100** Base class for builtin execution errors (*4100 - 4199*).
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 3299f80..d010cd9 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -22,8 +22,9 @@ Base classes for LDAP plugins.
import re
import json
+import time
-from ipalib import crud, errors
+from ipalib import api, crud, errors
from ipalib import Method, Object
from ipalib import Flag, Int, List, Str
from ipalib.base import NameSpace
@@ -175,6 +176,50 @@ def get_effective_rights(ldap, dn, attrs=None):
return rdict
+def wait_for_memberof(keys, entry_start, completed, show_command, adding=True):
+"""
+When adding or removing reverse members we are faking an update to
+object A by updating the member attribute in object B. The memberof
+plugin makes this work by adding or removing the me
Re: [Freeipa-devel] [PATCH] 635 wait for memberof plugin when doing reverse members
Jan Zelený wrote: Rob Crittenden 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. 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. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
