Giter Site home page Giter Site logo

Comments (12)

2ps avatar 2ps commented on July 17, 2024 20

This was really goofy, but what I did was override the celery base app's now to return the proper time, and it started working. There is a bug in celery v4.1.0 where Celery.now does not calculate the current time properly for non-UTC timezones.

class MyCelery(Celery)
    def now(self):
        """Return the current time and date as a datetime."""
        from datetime import datetime
        return datetime.now(self.timezone)

app = MyCelery()

And this fixed the issue for us. n.b., this issue may be fixed in the master branch, but it may not be the wisest decision to rely upon version 4.X getting timezones right anytime soon.

from django-celery-beat.

DDevine avatar DDevine commented on July 17, 2024 1

I was forced to look back into this - I have no idea why my other code was working in my development machine but is not working in new installs!

I traced it down to what you alluded to @JoshYuJump - your fix is not the most correct, but it is on the right track.

celery.app.base.now() was returning a PyTZ timezone, which Celery should use - however it does not make sense in the Django Celery Beat context. I think that's what the PyTZ Introduction section is alluding to (http://pytz.sourceforge.net/#introduction) however whenever I look at the PyTZ documentation my eyes instantly glaze over.

Anyway - for Django Celery Beat we need to localize self.app.now() in schedulers.ModelEntry._default_now().

I'll look testing my fix further tomorrow and submit a PR.

Without this #68 doesn't work.

from django-celery-beat.

DDevine avatar DDevine commented on July 17, 2024

This is related to #49

Giving the ability to configure nowfun makes sense.

But in the meantime defaulting to using django.utils.timezone.now for nowfun by adding it to the arguments of django_celery_beat.schedules.ModelEntry.from_entry it will solve the vast majority of use-cases in the short term and it seems to me that migrating to a more flexible solution later should be trivial.

At the moment the existing behaviour is useless for the vast majority of people, so at least having a sane default is a considerable upgrade.

from django-celery-beat.

RabidCicada avatar RabidCicada commented on July 17, 2024

Hey there @DDevine , Thanks for the research. I'll be hacking around to implement your workaround until this gets fixed the right way. Could you tell me exactly where in the from_entry line to put the nowfun arg? I'll be working on it tonight so I may get to it before you respond...but I may also get lost:).

I just got bit by this messing with my crontab tasks when I turned the timezone to the proper timezone instead of UTC

from django-celery-beat.

DDevine avatar DDevine commented on July 17, 2024

I've looked into it some more, I've found it a bit tricky to debug and to wrap my head around all the timezone stuff. There's a lot more problems, particularly where Solar is concerned - I'm not convinced it was ever tested before being added to Celery...

So firstly, from_entry was not the right place to put nowfun - I ended up adding it to models.py in CrontabSchedule.

    @property
    def schedule(self):
        nf = lambda: make_aware(now())
        return schedules.crontab(minute=self.minute,
                                 hour=self.hour,
                                 day_of_week=self.day_of_week,
                                 day_of_month=self.day_of_month,
                                 month_of_year=self.month_of_year, nowfun=nf)

This allows Celery to process the crontab in context of your local time - assuming that you have Django TIME_ZONE set to your local time. This is probably not the right way to do it - but I am just hacking at this problem for the moment.

TIME_ZONE = 'Australia/Brisbane' # Django TZ
CELERY_ENABLE_UTC = True # If false, when messages are sent they are kept in local time, if True they are converted to UTC.
CELERY_TIMEZONE = 'Australia/Brisbane'

Pytz is creating a LMT based timezone for Australia/Brisbane (+10:12 LMT) rather than a clean UTC offset (10:00 UTC) or tomezone abbreviation (... AEST). LMT does not cleanly convert to UTC and I found that when converting using pytz localize() it would drop the offset, so when converting you have to make sure you add (or subtract) the UTC offset.

For example, in the celery.schedules.solar.remaining_estimate():

        next = self.maybe_make_aware(next_utc.datetime())
        now = self.maybe_make_aware(self.now())
        next = localize(next, timezone.utc)
        offset = now.utcoffset()
        now = localize(now, self.tz)

        delta = next - now - offset
        return delta

It also seems that when there are Crontab schedules are not checked/executed while there are Solar schedules. Awesome...

Another problem is that I notice the delta goes negative. This issue is dealt with in the crontab class by using ffwd to only to positive deltas.

It also seems that upon restarting celerybeat solar tasks that have already been run are also run again... Maybe this is an issue with last_run_at? I'll have to look into specifics regarding last_run_at to know more.

The whole lot is a mess. I really didn't anticipate this trouble!

from django-celery-beat.

RabidCicada avatar RabidCicada commented on July 17, 2024

Holy Cow....You weren't kidding about tricky.

I dove in deep last night and...bleh.

So (separate bug), Celery is not properly taking django's TIME_ZONE setting like it says it will and it tries to do. It simply doesn't see django's timezone variable for some reason. So I had to explicitly set celey's timezone like you did.

You are right about the LMT thing. That would have driven me crazy had you not mentioned it. I wonder why it does that-->https://stackoverflow.com/questions/11473721/weird-timezone-issue-with-pytz.

I tracked a lot of the handling and just figured I could provide 'app=self.app' to some constructors since a lot of the is_due() stuff is mediated by ModelEntry.

But it looks like you know the problem I ran into next....a lot of the other calculations (remaining_delta, remaining_time etc)don't use app.now() directly and so I end up with offsets that are off. It does look like nowfun is the way to go...but I was looking for a way to not touch the django model itself.

Perhaps I still can go with using app.now() properly....looking into it more later. I spent a good deal of time last night hacking around celery, django-celery-beat, and my app tracing exactly where and how things are called.

from django-celery-beat.

RabidCicada avatar RabidCicada commented on July 17, 2024

If you have any more insights let me know:).

from django-celery-beat.

DDevine avatar DDevine commented on July 17, 2024

Yes using app.now() should be the right way to do it - because it abstracts away from Django by selecting nowfun or self.app.now() - and I can't see why it shouldn't work.

I adjusted my debug statements a little and found that crontab was using nowfun, but solar was not... This made me face palm because I realised that I had not told solar to use nowfun. Once I got that fixed in django-celery-beat (quite simple) it all came together, though I had to remove some of that localization/delta code I showed above and return to the default. I assume nowfun() is not using a LMT timezone and thus avoids those issues.

django-celery-beat SolarSchedule

    @property
    def schedule(self):
        nf = lambda: make_aware(now())
        return schedules.solar(self.event, self.latitude, self.longitude, nowfun=nf)

AND!!!
django-celery-beat CrontabSchedule

    @property
    def schedule(self):
        nf = lambda: make_aware(now())
        return schedules.crontab(minute=self.minute,
                                 hour=self.hour,
                                 day_of_week=self.day_of_week,
                                 day_of_month=self.day_of_month,
                                 month_of_year=self.month_of_year,
                                 nowfun=nf)

The question is - what is the real issue here? Is there more than one? Do we need to detect funky LMT datetimes and handle them over in celery? Does Celery try to use Django's TIME_ZONE? I was under the impression that Celery did not try to directly use Django's TIME_ZONE variable - and from what I can tell CELERY_TIMEZONE from within Django does seem to work. Were you getting confused between TIME_ZONE and CELERY_TIMEZONE?

from django-celery-beat.

RabidCicada avatar RabidCicada commented on July 17, 2024

From Celery's Periodic Tasks Notes

Django Users
Celery recommends and is compatible with the new USE_TZ setting introduced in Django 1.4.

For Django users the time zone specified in the TIME_ZONE setting will be used, or you can specify a custom time zone for Celery alone by using the timezone setting.

But it lies:).

Btw thanks for this investigation and the work to fix.

from django-celery-beat.

zh-h avatar zh-h commented on July 17, 2024

@RabidCicada Because that is old style project djcelery. Manually setting CELERY_TIMEZONE resolved my problem too. #5 (comment)

from django-celery-beat.

JoshYuJump avatar JoshYuJump commented on July 17, 2024

In the celery/app/base.py

    def now(self):
        """Return the current time and date as a datetime."""
        from datetime import datetime
        # return datetime.utcnow().replace(tzinfo=self.timezone)
        print(datetime.now())
        return datetime.now()

from django-celery-beat.

DDevine avatar DDevine commented on July 17, 2024

@JoshYuJump ...you put this ticket in the wrong repo? I thought I may have left that debug code in there but looking at my pull requests and celery master that code isn't there.

from django-celery-beat.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.