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

[Misnyovszki Adam]

On Wed, 09 Apr 2014 14:53:34 +0200
Petr Viktorin <pviktori at redhat.com> wrote:

> On 04/09/2014 01:45 PM, Petr Viktorin wrote:
> > 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
> >
> 
> I spoke too soon. There is one more doctest failure. This patch
> should fix it, can you review?
> 

works for me, thanks!
Adam