Fix Issue #604: Celery Beat Crashing at the End of Daylight Savings

[polarmt]

Dependencies

celery/celery#7901

Note: The two PRs are codependent and need to be merged together.

Overview

This is my suggested fix for #604. The following issue assumes America/Los_Angeles.

Testing

All testing was done on the following versions:
django: 3.2.15
celery: 5.2.7
django-celery-beat: 2.4.0

USE_TZ is not set.

Reproducing the Issue

In the Django shell (python manage.py shell):

from datetime import datetime, timedelta
from celery.utils import time
from django_celery_beat.utils import make_aware
from pytz import timezone

dt  = datetime(2022, 11, 6, 1, 34, 0)
now = make_aware(dt)

Without the changes in this PR, this should throw an AmbiguousTimeError.

Behavior of remaining During Transition (Without celery/celery#7901)

We can run the following script to test how the Celery beat will calculate the time remaining during the transition:

from datetime import datetime, timedelta
from celery.utils import time
from django_celery_beat.utils import make_aware
from pytz import timezone

dt  = datetime(2022, 11, 6, 1, 34, 0)
now = make_aware(dt)

tz = timezone('America/Los_Angeles')
start = datetime(2022, 11, 6, 1, 15, 0)
start = tz.localize(start, is_dst=True)
ends_in = timedelta(minutes=80)

print(time.remaining(time.maybe_make_aware(start), ends_in, time.maybe_make_aware(now)))

now here would be 1:34 AM PST or 1:34:00-8:00 while start would be 1:15 AM PDT or 1:15:00-7:00. The correct return value of time.remaining(...) here would be 1 minute since 1 hour 20 minutes from start would be 1:35:00-8:00. The problem is that time.remaining(...) returns 1 hour 1 minute because time.remaining will convert 1:15:00-7:00 to 1:15-8:00.

Behavior of remaining During Transition (With celery/celery#7901)

We can use the same script as the above subsection after making the changes in celery/celery#7901. This will cause time.remaining(...) to return 1 minute instead of 1 hour 1 minute, which is the correct value.

[auvipy]

beside it will be very useful to have proper unit tests for this change

[auvipy]

I am not sure about this part, can any other way we can check it?

[polarmt]

I referenced these posts 1 (top answer only) and 2. There are other suggested solutions online. I can summarize the problems with these solutions.

Use of localize

Source 1
Source 2
Source 3

The root cause was inside localize. Using the localize to fix the problem with localize sounds counterintuitive. For instance, link 1 mentions the following in the final solution:

Once you've **localized** a datetime in fact you can simply check it's .dst() method to see if it's zero.

time.daylight

Source

This solution might work, but the underlying code is just to do a similar check to what I do here. This code is cleaner, and I can change it if you would like.

Calculations

Source

One way is to calculate the timezone based on the current time and a previous time, but I think that this solution is long and too ad-hoc when we can achieve this with a few lines of code.

Conclusion

I think that time.daylight or time.localtime+tm_isdst is the best way to go for this. If you believe in the other solutions, please let me know.

[auvipy]

I have to check to give you proper answer

[polarmt]

Sounds good. Let me know when you've checked.

[polarmt]

beside it will be very useful to have proper unit tests for this change

I added the unit tests.

[cclauss]

Now that tests are passing on Django v5 this all becomes more difficult because:

• Support for pytz timezones is removed.
• The is_dst argument is removed from:
  • QuerySet.datetimes()
  • django.utils.timezone.make_aware()
  • [ ... ]

https://docs.djangoproject.com/en/5.0/releases/5.0/#features-removed-in-5-0

[cclauss]

@polarmt Please rebase to resolve the git conflicts.

[cclauss]

@polarmt Tests are ~50/50 pass/fail and in Django v5 the default value of the USE_TZ setting is changed from False to True.

Now that we have release v2.6.0 that supports Django v5, how should we proceed with this effort?