On Fri, 21 Mar 2014 10:33:00 +0100
Petr Viktorin <pvikt...@redhat.com> wrote:

> On 03/21/2014 10:29 AM, Petr Viktorin wrote:
> > On 03/20/2014 04:22 PM, Misnyovszki Adam wrote:
> >> On Thu, 20 Mar 2014 14:19:51 +0100
> >> Misnyovszki Adam <amisn...@redhat.com> wrote:
> >>
> >>> On Fri, 14 Mar 2014 13:26:15 -0400
> >>> Rob Crittenden <rcrit...@redhat.com> wrote:
> >>>
> >>>> Misnyovszki Adam wrote:
> >>>>> Hi,
> >>>>>
> >>>>> automember-rebuild uses asynchronous 389 task, and returned
> >>>>> success even if the task didn't run. This patch fixes this issue
> >>>>> adding a --nowait parameter to 'ipa automember-rebuild',
> >>>>> defaulting to False, thus when the script runs without it, it
> >>>>> waits for the 'nstaskexitcode' attribute, which means the task
> >>>>> has finished, according to
> >>>>> http://directory.fedoraproject.org/wiki/Task_Invocation_Via_LDAP#Implementation.
> >>>>>
> >>>>> Old usage can be enabled using --nowait.
> >>>>>
> >>>>> https://fedorahosted.org/freeipa/ticket/4239
> >>>>>
> >>>>> Request for comments:
> >>>>> - Should I add a parameter to specify the polling time? (now
> >>>>> 1ms)
> >>>>> - Should I add a parameter to specify the maximum polling
> >>>>> number? Now if something fails about creating the task, it
> >>>>> polls forever.
> >>>>> - Obviously, if these parameters should be added, there should
> >>>>> be a reasonable default for them (~ Required=False, Default=X).
> >>>>
> >>>> I don't think you need a polling time, esp since this is hidden
> >>>> from the user, but I think that is probably too short and you
> >>>> may end up hammering the LDAP server.
> >>>>
> >>>> I also wonder if there should be some maximum wait time. I don't
> >>>> like loops that can never exit. I'm at a loss for what that time
> >>>> should be though. And we'd need to spell out that we gave up
> >>>> waiting, not that the task necessarily failed. So rather than
> >>>> having a polling time option, rename nowait into wait_for=20, so
> >>>> wait for 20 seconds. Or something like that.
> >>>>
> >>>> I'd suggest using get_entry since you already know the full DN
> >>>> and there is only ever one. It would make this much more
> >>>> readable.
> >>>>
> >>>> I wonder if some top-level documentation should be added to flesh
> >>>> this out some more. This does, for example, return False in one
> >>>> case. The meaning for that should be spelled out.
> >>>>
> >>>> rob
> >>>
> >>> Hi,
> >>> personally I would keep --no-wait, with a delay of 1 seconds, and
> >>> a maximum waiting time for 60 seconds. Questions is, do we need to
> >>> parameterize with other parameters(wait-for to the maximum time,
> >>> and/or poll-delay for the delay time, both not required), or it is
> >>> not a case which worth to develop?
> >>> Greets
> >>> Adam
> >>
> >> Hi,
> >> here are the corrections Petr asked, also the other patch conatins
> >> the plugin registration refactor.
> >
> >
> > Thanks!
> >
> > You forgot an alternate summary for the --no-wait case. (Make sure
> > to output the DN in this case, so it's possible to poll for it.)
> >
> >
> >
> > Instead of `task['nstaskexitcode'][0]` please use
> > `task.single_value['nstaskexitcode']`.
> >
> > Here:
> >
> >     raise errors.DatabaseError(
> >         desc=_("Automember rebuild membership task failed"),
> >         info=_("nstaskexitcode = '%s'" %
> > str(task['nstaskexitcode'][0])))
> >
> > there's no need to call str() on %'s argument.
> > Also, use natural language (like "Task exit code: %s"), otherwise
> > there's no need to mark the message for translation.
> >
> >
> 
> And one more thing: Please bump the minor version in the VERSION file 
> when API.txt changes.
> 
> 

Hi,
everything is corrected!
Greets
Adam

>From 049a163583e18d63716a8419849b5f1880cb567f Mon Sep 17 00:00:00 2001
From: Adam Misnyovszki <amisn...@redhat.com>
Date: Fri, 21 Mar 2014 11:58:44 +0100
Subject: [PATCH] automember rebuild nowait feature added

automember-rebuild uses asynchronous 389 task, and returned
success even if the task didn't run. this patch fixes this
issue adding a --nowait parameter to 'ipa automember-rebuild',
defaulting to False, thus when the script runs without it,
it waits for the 'nstaskexitcode' attribute, which means
the task has finished. Old usage can be enabled using --nowait,
and returns the DN of the task for further polling.

https://fedorahosted.org/freeipa/ticket/4239
---
 API.txt                      |  3 ++-
 VERSION                      |  4 ++--
 ipalib/plugins/automember.py | 56 ++++++++++++++++++++++++++++++++++++--------
 3 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/API.txt b/API.txt
index 8e1f7713ade2b3dc046e9db82fdd6be2d85eec56..fa28a88f78270dd5858d97b88ee22ae7cdd8b13c 100644
--- a/API.txt
+++ b/API.txt
@@ -201,8 +201,9 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('value', <type 'unicode'>, None)
 command: automember_rebuild
-args: 0,4,3
+args: 0,5,3
 option: Str('hosts*')
+option: Flag('no_wait?', autofill=True, default=False)
 option: StrEnum('type', cli_name='type', multivalue=False, required=False, values=(u'group', u'hostgroup'))
 option: Str('users*')
 option: Str('version?', exclude='webui')
diff --git a/VERSION b/VERSION
index 4f01e38c0c3a52ae6293e458fdb4d3e0a14aa8aa..abf23e3c3b21357a9a70a9bfd2a7751c8272de9b 100644
--- a/VERSION
+++ b/VERSION
@@ -89,5 +89,5 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=78
-# Last change: pviktori - permission extratargetfilter
+IPA_API_VERSION_MINOR=79
+# Last change: amisnyov - automember nowait
diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py
index a12bfb52522e38bc083d0750dc66c894a4aeba53..95c77b40076dc4e1b6cce6fc5f18da4e91b83ef9 100644
--- a/ipalib/plugins/automember.py
+++ b/ipalib/plugins/automember.py
@@ -17,7 +17,10 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 import uuid
+import time
+
 import ldap as _ldap
+
 from ipalib import api, errors, Str, StrEnum, _, ngettext
 from ipalib.plugins.baseldap import *
 from ipalib.request import context
@@ -623,9 +626,14 @@ class automember_rebuild(Command):
             label=_('Hosts'),
             doc=_('Rebuild membership for specified hosts'),
         ),
+        Flag(
+            'no_wait?',
+            default=False,
+            label=_('No wait'),
+            doc=_("Don't wait for rebuilding membership"),
+        ),
     )
     has_output = output.standard_value
-    msg_summary = _('Automember rebuild membership task completed')
 
     def validate(self, **kw):
         """
@@ -693,20 +701,48 @@ class automember_rebuild(Command):
         else:
             search_filter = '(%s=*)' % obj.primary_key.name
 
+        task_dn = DN(
+            ('cn', cn),
+            ('cn', 'automember rebuild membership'),
+            ('cn', 'tasks'),
+            ('cn', 'config'))
+
         entry = ldap.make_entry(
-            DN(
-                ('cn', cn),
-                ('cn', 'automember rebuild membership'),
-                ('cn', 'tasks'),
-                ('cn', 'config'),
-            ),
+            task_dn,
             objectclass=['top', 'extensibleObject'],
             cn=[cn],
             basedn=[basedn],
             filter=[search_filter],
-            scope=['sub']
-        )
+            scope=['sub'])
         ldap.add_entry(entry)
-        return dict(result=True, value=u'')
+
+        if options.get('no_wait'):
+            summary = u"Automember rebuild membership task started\n" \
+                + "DN=%s" % task_dn
+        else:
+            summary = u'Automember rebuild membership task completed'
+            start_time = time.time()
+            while True:
+                try:
+                    task = ldap.get_entry(task_dn)
+                except errors.NotFound:
+                    break
+
+                if 'nstaskexitcode' in task:
+                    if str(task['nstaskexitcode'][0]) == '0':
+                        break
+                    else:
+                        raise errors.DatabaseError(
+                            desc=_("Automember rebuild membership task failed"),
+                            info=_("Task exit code = '%s'" % task.single_value['nstaskexitcode']))
+                time.sleep(1)
+                if time.time() > (start_time + 60):
+                   raise errors.DatabaseTimeout()
+
+
+        return dict(
+            result=True,
+            value=u'',
+            summary=summary)
 
 api.register(automember_rebuild)
-- 
1.8.5.3

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to