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

[Petr Viktorin]

On 04/09/2014 01:43 PM, Misnyovszki Adam wrote:
> On Tue, 08 Apr 2014 17:31:25 +0200
> Petr Viktorin <pviktori at redhat.com> wrote:
>
>> 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.
>>
> Hi,
> corrected.
> Greets
> Adam
>

ACK
Pushed to master: 3f61bbaef582ff42b151f2bb01f312a94a70632c

-- 
Petr³