Error When an Invalid Recurring Task is Configured

[jherdman]

I took a first crack at resolving #414. I think it's a little bit of a blunt solution.

Resolves #414

[rosa]

Thanks a lot @jherdman! I was thinking that perhaps this should be part of a more generic configuration validation that runs and raises before starting the supervisor 🤔

[jherdman]

Perhaps we need both things? Since the recurring jobs are just DB backed, and have validations, we probably need to report on those validation errors should they occur.

[rosa]

we probably need to report on those validation errors should they occur.

Right, I mean doing this as part of that configuration validation that runs before the supervisor starts, instead of waiting until we try to schedule them.

[jherdman]

I can take a crack at your suggestion unless you have something very specific in mind.

[rosa]

@jherdman thank you! I was just thinking of having the current check here expanded to also check if the configuration is valid (configuration.valid?), and if not, abort printing the existing errors. I think not having any configured processes could be part of the configuration.valid? check.

[jherdman]

@rosa take two! I think this is more in line with what you noted above.

[jherdman]

I feel like this is probably not exhaustive enough, but I'm unsure of other cases to expand upon. This is probably sufficient for the scope of this PR though.

[rosa]

Could we include perhaps the case of no configured processes? So we could unify that call in the Supervisor, instead of having two separate checks?

Besides that, I think you can skip recurring_tasks.none? because if recurring_tasks is empty, recurring_tasks.all?(&:valid?) will return true.

[jherdman]

Ugh... seems so obvious after you pointed it out 😅

I hope you don't mind, but I've kept the two distinct error messages. It feels like the verbose invalid task error message would be helpful.

[rosa]

Wonderful! Thank you so much, @jherdman! 🙏 🙇‍♀️