Fix recurring tasks not running within timezone shadow

[emyller]

The following test usually passes under daylight. Right now — it's evening — after glancing at the code, my best guess is that the time zone somehow affects behavior.

tests/unit/task_processor/test_unit_task_processor_models.py::test_recurring_task_run_should_execute_first_run_at[first_run_time1-False] FAILED       [1/1]
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> traceback >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

first_run_time = datetime.time(0, 44, 41, 506850), expected = False

    @pytest.mark.parametrize(
        "first_run_time, expected",
        ((one_hour_ago.time(), True), (one_hour_from_now.time(), False)),
    )
    def test_recurring_task_run_should_execute_first_run_at(
        first_run_time: time,
        expected: bool,
    ) -> None:
>       assert (
            RecurringTask(
                first_run_time=first_run_time,
                run_every=timedelta(days=1),
            ).should_execute
            == expected
        )
E       assert True == False
E        +  where True = <RecurringTask: RecurringTask object (None)>.should_execute
E        +    where <RecurringTask: RecurringTask object (None)> = RecurringTask(first_run_time=datetime.time(0, 44, 41, 506850), run_every=datetime.timedelta(days=1))
E        +      where datetime.timedelta(days=1) = timedelta(days=1)

tests/unit/task_processor/test_unit_task_processor_models.py:65: AssertionError

[emyller]

I finally have more context on this.

The issue is caused by the attribute RecurringTask.first_run_time being a TimeField. Hence the following condition fails whenever now is between 00:00 UTC and 01:00 UTC, when first_run_time is 23:** and now is 00:**.

flagsmith-common/src/task_processor/models.py

Line 192 in 7ac2e3b

return not (self.first_run_time and self.first_run_time > now.time())

Need to discuss with the team whether we want to try and do some [temporary] math or go ahead and fix the underlying issue by making RecurringTask.first_run_time a DateTimeField, so that the operator > will make sense.

IMO this is worth elevating priority if we're relying on any scheduled task within that range.
cc @matthewelwell @khvn26 @gagantrivedi

[matthewelwell]

Thanks for the context on this one @emyller.

So, if I understand it correctly, this will only affect the first run of a given recurring task, if we schedule it to run between 00:00 and 01:00 UTC.

the issue with using a DateTimeField here, is that we don't always know which date a certain feature might be released on, resulting in some mildly inconvenient release coordination etc. I'd like to see if there is another option that allows us to keep the field as a time field, but handles the logic better. Maybe your '[temporary] math' suggestion doesn't need to be temporary?

[emyller]

Thanks for explaining how this affects the product. I thought it was a minor issue then, it seems even smaller now.

Yeah I think we can solve the problem without a date. Gotta try and make it pretty though. :)

[khvn26]

What do you think about reworking recurring tasks to use crontab expressions, maybe in conjunction with pg_cron?