Follow-up from "[NDF] Implement TODO about thread safety in churn limiter"
The following discussions from !4526 (merged) should be addressed:
-
@pjbrone started a discussion: // managedMarkContractUtility checks an active contract in the contractor and
-
@pjbrone started a discussion: (+1 comment) // it may just be this host that has an issue.
I understand why your doing this, and I also see how you can't get out of doing it. It's just not ideal right. I'm worried ppl are going to extend this function and do
if err != nil {return err}
without thinking twice about it, not realizing they break the outer loop.Does it make sense perhaps to signal this through an extra
bool
return var? Or add aNOTE
in the docstring? -
@pjbrone started a discussion: as suggested earlier, perhaps we can add an extra bool and do it this way:
if fatal { return err }
not sure if I'm terribly fond of this either but not doing it leaves room for future error imo