On 03/21/2014 12:58 PM, Martin Kosek wrote:
On 03/21/2014 12:38 PM, Petr Viktorin wrote:
On 03/21/2014 12:00 PM, Misnyovszki Adam wrote:
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']`.
There's one more `task['nstaskexitcode'][0]`. (Perhaps putting it in a variable
would be better?)
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
Sorry for dragging this so long, but:
The DN should not be a part of the summary, because that makes it hard to parse
when using the API. It should be returned as a part of the result.
To do that, you'd need to change the output type:
has_output = output.standard_entry
has_output_params = (
DNParam(
'dn',
label=_('Task automember'),
doc=_('DN of the started task'),
),
)
and provide a dict in result, instead of True. (And with --no-wait, add the dn
to that dict.)
Do really want to use 'dn' for the DNParam? dn is already used for standard
entry DN when --all is used, right? "automembertaskdn" may be better.
Well, I think "DN of the added entry" is exactly what this is.
Also, "Task automember" label sounds strange to me, would "Automember task DN"
be better?
I meant "Task DN", sorry for the thinko.
--
Petr³
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
