Skip to content

Adjust weekday behavior of JobQueue.run_daily #3038

@Bibo-Joshi

Description

@Bibo-Joshi

Current situation

Currently, 0 correspondigs to monday and 6 to sunday. This does not match the the behavior of cron, which is the de-facto standard for many scheduling related things in IT. In cron, we have 0=sunday, 6=saturday. Moreover, the APScheduler library, which we currently use as backend for the JobQueue will also adapt the sun-sat approach in the next major release 4.0 - cf agronholm/apscheduler#495

Proposed change

We should make the transition to a sun-sat based numbering scheme. This has the advantages of

  • complying with cron
  • having one less thing to worry about for users when we switch to APS 4.0 at some point in the future

An implementation could for example use a mapping like here to pass the correct values to APS.
Accepting the weekdays as mon, tue etc instead of integers could also be added as additional support, but is not mandatory IMO.

A PR for this issue should

  • Implement a conversion of the input such that we accept 0=sun, 6=sat, but pass the correct values to APS
  • add a .. versionchanged:: 20.0 directive to the docstring of run_daily and the days parameter to explain the changed behavior
  • Add a unit test to test_jobqueue.py that ensures that this works as expected
  • Make JobQueue.run_daily issue a warning that tells users about the changed behavior and recommends them to double check that they pass the correct days. This warning should be issued via the warn function from telegram._utils.warnings. The unit test should ensure that this warning is issued correctly. See here for an example on how warnings can be checked in unit tests.

Additional Info

We have a contribution guide

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions