Skip to content

fix: Accept expires as alias for expire_seconds#1018

Open
serl wants to merge 3 commits into
celery:mainfrom
serl:beat-config-expires
Open

fix: Accept expires as alias for expire_seconds#1018
serl wants to merge 3 commits into
celery:mainfrom
serl:beat-config-expires

Conversation

@serl

@serl serl commented Apr 14, 2026

Copy link
Copy Markdown

While reconfiguring some beat tasks I have in the django settings, I stumbled upon the expiration of those.

According to the Celery docs on tasks, I can pass expires to apply_async to avoid tasks being ran after some deadline.

According to the Celery doc on periodic tasks, I can pass this kind of arguments in the options field. expires is even an example in the doc.

It works indeed when using the default CELERY_BEAT_SCHEDULER, but for django-celery-beat I had to use expire_seconds instead.

Here's a snippet of my current configuration to make it work across all schedulers:

CELERY_BEAT_SCHEDULE = {
    "debug_heartbeat": {
        "task": "debug_task",
        "schedule": 10,
        "options": {
            "expires": 1, # works with the default scheduler
            "expire_seconds": 1, # works only with django-celery-beat scheduler
        },
    },
}

With this PR, expires works with django-celery-beat scheduler as well

@codecov

codecov Bot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.67%. Comparing base (edb45fc) to head (7884c7a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1018      +/-   ##
==========================================
+ Coverage   87.64%   87.67%   +0.02%     
==========================================
  Files          32       32              
  Lines        1012     1014       +2     
  Branches       81       82       +1     
==========================================
+ Hits          887      889       +2     
  Misses        107      107              
  Partials       18       18              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for configuring task expiration in CELERY_BEAT_SCHEDULE using Celery’s expires option while persisting it to django-celery-beat’s expire_seconds field.

Changes:

  • Map options["expires"] (numeric) to expire_seconds when unpacking scheduler options.
  • Add unit tests asserting the alias behavior and precedence when both keys are provided.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
django_celery_beat/schedulers.py Adds expires handling in _unpack_options to populate expire_seconds.
t/unit/test_schedulers.py Adds tests for expiresexpire_seconds mapping and precedence rules.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread django_celery_beat/schedulers.py Outdated
Comment thread django_celery_beat/schedulers.py
Comment thread t/unit/test_schedulers.py Outdated
Comment thread t/unit/test_schedulers.py
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@cclauss

cclauss commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

bool is a subclass of int in Python.

This is nonsense.

I have to say that I really dislike that expire_seconds can be None. Why make it polymorphic with None?

expire_seconds should be a nonnegative int. I would not even allow it to be a float unless we are going to support partial seconds.

if expire_seconds:
    do_something()

@serl

serl commented Apr 16, 2026

Copy link
Copy Markdown
Author

bool is a subclass of int in Python.

This is nonsense.

I have to say that I really dislike that expire_seconds can be None. Why make it polymorphic with None?

expire_seconds should be a nonnegative int. I would not even allow it to be a float unless we are going to support partial seconds.

if expire_seconds:
    do_something()

I guess there's a difference between "not given by the user" and "0". Not really an expert, but I'd guess that "not given by the user" means "infinity" and "0" means "we'll never run this task".

By the way, this is very much out of the scope of this PR in my opinion.
The scope was "the advertised name for this thing in the Celery doc is expires, while in this repository is expire_seconds. Please support both"

@cclauss

cclauss commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

"not given by the user" means "infinity" and "0" means "we'll never run this task".

Like you say, "never run this task" makes no sense (is a footgun), so 0 means infinity (never expires), and there is no need for None.

@serl

serl commented Apr 29, 2026

Copy link
Copy Markdown
Author

I ran some more experiments against the default CELERY_BEAT_SCHEDULER, not this one:

  • passing a timedelta in expires simply does not enqueue anything. So for me it's undefined (maybe buggy?) behavior at Celery level, not something we should take into account in this specific repository
  • passing 0 indeed makes the task being revoked immediately. Foot-gun-y indeed

IMO, this PR does what it's supposed to do in this specific repository. There's some rough corners that are part of celery/the default beat implementation that might/should be tackled elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants