On Tue, Mar 4, 2014 at 9:12 AM, Thomas Thrainer <[email protected]> wrote: > I think that's not needed. The return value of the method (lu_result) is > correctly updated if there are results from nodes. The case which happened > in issue 742 was that an offline node was incorrectly contacted. However, > the result of this attempt didn't make it in the return value of > HooksCallBack(...), because in line 3252 the result of offline nodes is > discarded. So while fixing issue 742, this instances of the bug should go > away. > > However, the result of the node is also discarded if it gave an error > message (also checked in line 3252). This sounds fishy to me, as the return > value should be False as well if the RPC call resulted in an error... So > maybe something like this would make more sense: > > if test: # test is defined in line 3249 > lu_result = False > continue > if res.offline: > continue > > What do you think? > > Cheers, > Thomas
I think that both approaches can work. In either cases, when an error is detected, self._ErrorIf is invoked, and that function sets self.bad. So, using "not self.bad" should be enough. My favorite approach would be to not use lu_result at all in the function, and just use it in the return statement to propagate the previous Exec result, but your approach is ok as well, so I'll send and interdiff to implement it. Thanks, Michele -- Google Germany GmbH Dienerstr. 12 80331 München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores
