8000 The time set in run_daily shifts over weeks · Issue #1338 · python-telegram-bot/python-telegram-bot · GitHub
[go: up one dir, main page]

Skip to content

The time set in run_daily shifts over weeks #1338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MarcelBeining opened this issue Feb 5, 2019 · 6 comments
Closed

The time set in run_daily shifts over weeks #1338

MarcelBeining opened this issue Feb 5, 2019 · 6 comments
Milestone

Comments

@MarcelBeining
Copy link

In my bot I have a run_daily job which should execute each Monday night at 0:01 however it runs at some arbitrary time in the afternoon after some weeks. The server clock is correct.
I think this is because even if you set a time point in run_daily, an interval is subsequently calculated by the jobqueue class and if the job's called function takes some time to complete or the job cannot be run immediately as other jobs are running, the job is afterwards put again into the queue but with some shift! This should be fixed, e.g. that the exact time point is stored in the job object and the interval at which it has to be called again is then calculated each time.

Steps to reproduce

  1. Create a function that takes a long time to finish. (30min+)

  2. Put that function into the jobQueue using run_daily and set a specific time point at which it should run each day.

  3. Let it run for some days.

Expected behaviour

The job should run each day at exactly the same time.

Actual behaviour

The time at which the job runs is slowly shifting to later time points.

Configuration

Operating System:
Ubuntu

Version of Python, python-telegram-bot & dependencies:
Python 3.6, python-telegram-bot v11.1.0

@Eldinnie
Copy link
Member

Hey,

I now see that we never responded to this issue. We've discussed it internally and this should definitely be fixed. Do you feel up to making a PR for that yourself?

@MarcelBeining
Copy link
Author

Hi @Eldinnie .
Hm I checked the code of JobQueue thoroughly. It seems the next time of the job is correctly set because the tick method hands over the scheduled timestamp of the job (last_t = t line 269) and to that timestamp the interval is added.
So the only other reason why this happens could be jobs which run longer than their interval time (or am I missing something?). The question is what should happen in that case?

  1. Leave as is because it's the users fault.
  2. Throw out a warning about it.
  3. If job.repeating is True but the new timestamp is already smaller than the current timestamp (time.time() ), add as many intervals until the jobs next time stamp is larger than the current time.

I would go for the third solution, but I do not know if there could be any kind of job which suffers from such a rule. What do you think? One could also make this rule optional (some kwarg like skipElapsedIntervals )

@MarcelBeining
Copy link
Author

Hi @Eldinnie @jsmnbom @leandrotoledo ,
I never got an answer on my last post, and my programs still suffer from this.
Would be cool to fix this soon.

@plammens
Copy link
Contributor
plammens commented Sep 2, 2019

I think the best solution would be to make each job run on a separate thread, so that even if the job takes longer than its interval to execute, it doesn't block the JobQueue flow, so that timings aren't altered.

So the only other reason why this happens could be jobs which run longer than their interval time

I also would think this is the culprit, but then it seems strange it's affecting you, since your job is scheduled for every week. @MarcelBeining can you confirm/deny that your job can take over a week to execute?

@Bibo-Joshi Bibo-Joshi added this to the 13.0 milestone Nov 18, 2019
@Bibo-Joshi
Copy link
Member

Hi. So, this issue hasn't gotton any attention for a long time but as we're now refactoring the JobQueue to rely on a third party lib, it should be fixed. I actually ran the example provided by @matveyplevako and it works fine as long as you set max_instances>1 in the job_kwargs.
It would be a great help if you could try and run your currently non-working examples on the v13 branch and report back it the new JobQueue does the trick :)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants
@MarcelBeining @Eldinnie @jsmnbom @plammens @Bibo-Joshi and others
0