-
Notifications
You must be signed in to change notification settings - Fork 6k
Closed
Labels
ℹ️ good-first-issueinformation: good-first-issueinformation: good-first-issue🛠 breakingchange type: breakingchange type: breaking
Milestone
Description
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.0directive to the docstring ofrun_dailyand thedaysparameter to explain the changed behavior - Add a unit test to
test_jobqueue.pythat ensures that this works as expected - Make
JobQueue.run_dailyissue 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 thewarnfunction fromtelegram._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
Labels
ℹ️ good-first-issueinformation: good-first-issueinformation: good-first-issue🛠 breakingchange type: breakingchange type: breaking