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

[Petr Viktorin]

On 04/08/2014 04:17 PM, Misnyovszki Adam wrote:
> On Mon, 07 Apr 2014 09:43:10 +0200
> Petr Viktorin <pviktori at redhat.com> wrote:
>
>> On 03/27/2014 03:37 PM, Misnyovszki Adam wrote:
>>> On Wed, 26 Mar 2014 13:15:55 +0100
>>> Petr Viktorin <pviktori at redhat.com> wrote:
>> [...]
>>>>
>>>> Looks great! I'm just concerned about the error returned when the
>>>> task takes too long:
>>>>        $ ipa automember-rebuild --type group
>>>>        ipa: ERROR: LDAP timeout
>>>> I don't think it's sufficiently clear from this that waiting for
>>>> the task timed out, but the task was actually started
>>>> successfully. A custom error with a more descriptive message would
>>>> be useful.
>>>>
>>>>
>>>> Also I've noticed that the "nstaskstatus" of a successful task is:
>>>>        Automember rebuild task finished. Processed (1) entries.
>>>> This looks helpful; we could return it as the summary.
>>>>
>>>
>>> Hi,
>>> both fixed.
>>> Greets
>>> Adam
>>>
>>
>> Sorry for the delay!
>> 'Automember' is a translatable string, so please wrap it in _() when
>> raising TaskTimeout. Also please update the tests.
>> Otherwise with a little rebase it's good to go.
>>
>>
>
> Hi,
> see the attached modifications, tests corrected, and added for no-wait,
> also rebased for current master.
> Greets
> Adam
>

Looks good overall, but why do you now set `self.msg_summary`? Keep in 
mind that currently the same Command object is reused for every 
automember_rebuild command, including commands that run in parallel in 
different threads. It should never be modified.

-- 
Petr³