Erroring out if retry is enabled

[maltoe]

Hey

I was wondering whether it would be a good change to refuse scheduling if the retry option of sidekiq is not set to false. Currently, if retry: true is set, you might end up with multiple jobs running at the same time (one scheduled, one or more retried by Sidekiq). This behaviour seems bad, and my guess is that all users of sidekiq-repeat have retry: false on their jobs.

Or at least issue a warning?

[dsander]

Currently, if retry: true is set, you might end up with multiple jobs running at the same time (one scheduled, one or more retried by Sidekiq).

I think that is something that should be taken care of by sidekiq-unique-jobs or a similar gem. There are scenarios where retry: true for a recurring job is useful. One could try to hit an external service every hour, but still retry it a few times if the service is down or flaky at the time of the execution of the job.
We could try to be smart and output a warning if the amount of retries configured would make the job retry longer than the configured recurrence.

[bigzed]

We could try to be smart and output a warning if the amount of retries configured would make the job retry longer than the configured recurrence.

In general I agree with @dsander, but the smartest solution would probably be to not only issue a warning, but refuse to retry if the next retry would be after the next scheduled job.
Retries which happen before the next scheduled job, would of course be executed.

[maltoe]

@bigzed That would be smart, but I wonder if we can manipulate Sidekiq's retry mechanism at all from within the middleware.

[dsander]

I am not even sure if the calculation would work in all cases. If parse-cron supports crazy cronlines we would need to brute force the shortest possible time-frame between two runs of the passed in cronline 😄

[maltoe]

Maybe we simply do nothing about it, but mention it in the README as well?

Make sure to turn of retry or at least make sure it fades off before the next scheduled time