On Wed, 09 Apr 2014 14:53:34 +0200 Petr Viktorin <pvikt...@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 <pvikt...@redhat.com> wrote: > >> > >>> On 04/08/2014 04:17 PM, Misnyovszki Adam wrote: > >>>> On Mon, 07 Apr 2014 09:43:10 +0200 > >>>> Petr Viktorin <pvikt...@redhat.com> wrote: > >>>> > >>>>> On 03/27/2014 03:37 PM, Misnyovszki Adam wrote: > >>>>>> On Wed, 26 Mar 2014 13:15:55 +0100 > >>>>>> Petr Viktorin <pvikt...@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 _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel