On Fri, 21 Mar 2014 10:33:00 +0100 Petr Viktorin <[email protected]> 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 <[email protected]> wrote: > >> > >>> On Fri, 14 Mar 2014 13:26:15 -0400 > >>> Rob Crittenden <[email protected]> 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 <[email protected]> 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 [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
