[Freeipa-devel] [PATCH][RFC] 7 automember rebuild nowait feature added

[Martin Kosek]

On 03/14/2014 06:24 PM, Petr Viktorin wrote:
> On 03/14/2014 05:31 PM, 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
> 
> Thanks for the patch!
> 
>> Request for comments:
...
> A subjective style nitpick: Using `while` with a constant is creative, but
> maybe it would be more readable to spell out if+while True.
> 
>> +            tasks = ldap.get_entries(
>> +                DN(
>> +                    ('cn', cn),
>> +                    ('cn', 'automember rebuild membership'),
>> +                    ('cn', 'tasks'),
>> +                    ('cn', 'config'),
>> +                ),
>> +            )
> 
> You can use ldap.get_entry to get a single entry. Both ldap.get_entry and
> ldap.get_entries will raise a NotFound exception if no entries are found,
> you'll need to handle that.

Additionally, that DN is already defined few lines above. I would rather have
that in a variable and used twice than having it constructed twice.

Martin