Refactor JobQueue #1981

Bibo-Joshi · 2020-06-05T21:28:00Z

When ready, closes #1936 and on the fly closes #2004

Introduces APScheduler as 3rd party lib for scheduling tasts. The PR is not yet really polished, especially I'll need to increase coverage. But before getting started on that, I'd like to get feedback on the general design.

My main concern is the Job (already mentioned offline). Unfortunately, simply subclassing aps.Job to add a context argument doesn't work - we'd have to patch aps.Job to do that. I don't fancy that.
On the other hand, having a custom Job class with custom attributes allows to make switching to another lib a bit less breaking, if we have to do that at some point.
So I added a job attribute to our Job class, which holds the corresponding aps.Job.

Besides the fact that rescheduling jobs must now be done directly through the aps interface, the main breaking change is, that JobQueue now can only handle pytz timezones. But personally, I don't see a problem in that. Switching from fixet utc-offset timezones should be easy enough and people who need weird custom timezones should be capable enough of handling the change …

Reragrding tests:

test_official currently fails b/c API 4.9
made the timeout tests for ConversationHandler a bit more generous on the timing, aps seems to like that
switched to pytzs everywhere in the tests
besides that, I only made the existing tests run. as mentioned above, I'll need to improve coverage, once we're okay with the general design.

EDIT:
That are a lot more failing tests than I'd hoped for … Will need to check on an actual OS locally (yes, I developed on Windows, shame on me …)

EDIT 2:

the updater.idle() test just wasn't executed on windows … fixed it.
APS is a bit pickier on doing things "right now". That's one of the things, we should mention on the release notes. So I just removed test_run_repeating_first_immediate and test_run_repeating_first_timezone is now also adapted for APS' needs

# Conflicts: # telegram/ext/jobqueue.py

# Was intended for interal use anyways # Fixes tests

Poolitzer

Looks good to me, I love outsourcing the jobs to the APS.

telegram/ext/jobqueue.py

Poolitzer · 2020-06-24T08:16:57Z

tests/test_conversationhandler.py

@@ -742,8 +735,7 @@ def test_conversation_handler_timeout_state_context(self, cdp, bot, user1):
        message.text = '/startCoding'
        message.entities[0].length = len('/startCoding')
        cdp.process_update(Update(update_id=0, message=message))
-        sleep(0.5)
-        cdp.job_queue.tick()
+        sleep(0.6)


doesnt it make sense to put this in a constant?

Not sure, if that's a good idea. Have one of the tests failing in the future, fiddle with the constant, suddenly everthing fails and it's not obvious why …

…am-bot/python-telegram-bot into refactor-job-queue

tests/test_jobqueue.py

jh0ker · 2020-06-24T19:15:07Z

telegram/ext/jobqueue.py

+            return self._tz_now() + time
+        if isinstance(time, datetime.time):
+            dt = datetime.datetime.combine(datetime.datetime.now().date(), time,
+                                           tzinfo=time.tzinfo or self.scheduler.timezone)


I don't know about this... Are we dropping 3.5 support (or did we already)? The tzinfo argument to combine has been added in 3.6.

Apart from that, couldn't datetime.datetime.now().date() return the wrong date this way?

Uh, you're right. Couldn't tell, because TG messed up our tests again …. But we're dropping py3.5 an #1920 anyway, so we might as well do here.

And right once more. I'll add it back.

…python-telegram-bot into refactor-job-queue

…r-job-queue

Bibo-Joshi · 2020-07-10T11:11:15Z

py3.7 fail unrelated. Codacy can't check for some reason, but I ran pre-commit locally again and it's fine.
As discussed offline we'll probably need to do some clean up after release anyway, but we need people to actaully use the new JobQueue for that. Merging.

* First go on refactoring JobQueue * Temporarily enable tests for the v13 branch * Work on tests * Temporarily enable tests for the v13 branch * Increase coverage * Remove JobQueue.tick() # Was intended for interal use anyways # Fixes tests * Address review * Temporarily enable tests for the v13 branch * Address review * Dispatch errors * Fix handling of job_kwargs * Remove possibility to pass a Bot to JobQueue

* Refactor handling of `default_quote` * Make it a breaking change * Pickle a bots defaults * Temporarily enable tests for the v13 branch * Temporarily enable tests for the v13 branch * Refactor handling of kwargs in Bot methods (#1924) * Unify kwargs handling in Bot methods * Remove Request.get, make api_kwargs an explicit argument, move note to head of Bot class * Fix test_official * Update get_file methods * Refactor JobQueue (#1981) * First go on refactoring JobQueue * Temporarily enable tests for the v13 branch * Work on tests * Temporarily enable tests for the v13 branch * Increase coverage * Remove JobQueue.tick() # Was intended for interal use anyways # Fixes tests * Address review * Temporarily enable tests for the v13 branch * Address review * Dispatch errors * Fix handling of job_kwargs * Remove possibility to pass a Bot to JobQueue * Refactor persistence of Bot instances (#1994) * Refactor persistence of bots * User BP.set_bot in Dispatcher * Temporarily enable tests for the v13 branch * Add documentation * Add warning to Updater for passing both defaults and bot * Address review * Fix test

* First go on refactoring JobQueue * Temporarily enable tests for the v13 branch * Work on tests * Temporarily enable tests for the v13 branch * Increase coverage * Remove JobQueue.tick() # Was intended for interal use anyways # Fixes tests * Address review * Temporarily enable tests for the v13 branch * Address review * Dispatch errors * Fix handling of job_kwargs * Remove possibility to pass a Bot to JobQueue

* Refactor handling of `default_quote` * Make it a breaking change * Pickle a bots defaults * Temporarily enable tests for the v13 branch * Temporarily enable tests for the v13 branch * Refactor handling of kwargs in Bot methods (#1924) * Unify kwargs handling in Bot methods * Remove Request.get, make api_kwargs an explicit argument, move note to head of Bot class * Fix test_official * Update get_file methods * Refactor JobQueue (#1981) * First go on refactoring JobQueue * Temporarily enable tests for the v13 branch * Work on tests * Temporarily enable tests for the v13 branch * Increase coverage * Remove JobQueue.tick() # Was intended for interal use anyways # Fixes tests * Address review * Temporarily enable tests for the v13 branch * Address review * Dispatch errors * Fix handling of job_kwargs * Remove possibility to pass a Bot to JobQueue * Refactor persistence of Bot instances (#1994) * Refactor persistence of bots * User BP.set_bot in Dispatcher * Temporarily enable tests for the v13 branch * Add documentation * Add warning to Updater for passing both defaults and bot * Address review * Fix test

* First go on refactoring JobQueue * Temporarily enable tests for the v13 branch * Work on tests * Temporarily enable tests for the v13 branch * Increase coverage * Remove JobQueue.tick() # Was intended for interal use anyways # Fixes tests * Address review * Temporarily enable tests for the v13 branch * Address review * Dispatch errors * Fix handling of job_kwargs * Remove possibility to pass a Bot to JobQueue

* Refactor handling of `default_quote` * Make it a breaking change * Pickle a bots defaults * Temporarily enable tests for the v13 branch * Temporarily enable tests for the v13 branch * Refactor handling of kwargs in Bot methods (#1924) * Unify kwargs handling in Bot methods * Remove Request.get, make api_kwargs an explicit argument, move note to head of Bot class * Fix test_official * Update get_file methods * Refactor JobQueue (#1981) * First go on refactoring JobQueue * Temporarily enable tests for the v13 branch * Work on tests * Temporarily enable tests for the v13 branch * Increase coverage * Remove JobQueue.tick() # Was intended for interal use anyways # Fixes tests * Address review * Temporarily enable tests for the v13 branch * Address review * Dispatch errors * Fix handling of job_kwargs * Remove possibility to pass a Bot to JobQueue * Refactor persistence of Bot instances (#1994) * Refactor persistence of bots * User BP.set_bot in Dispatcher * Temporarily enable tests for the v13 branch * Add documentation * Add warning to Updater for passing both defaults and bot * Address review * Fix test

First go on refactoring JobQueue

ac6046c

Bibo-Joshi added the pending review label Jun 5, 2020

Bibo-Joshi added this to the 13.0 milestone Jun 5, 2020

Bibo-Joshi requested a review from jh0ker June 5, 2020 21:28

This was referenced Jun 5, 2020

Add last keyword argument to run_repeating #1497

Closed

imlemented crontab based repeating job for jobqueue #1478

Closed

Bibo-Joshi changed the base branch from master to v13 June 6, 2020 10:55

Bibo-Joshi added 3 commits June 6, 2020 14:49

Temporarily enable tests for the v13 branch

bc9b05c

Merge remote-tracking branch 'origin/v13' into refactor-job-queue

522558d

Work on tests

2eb198e

Bibo-Joshi force-pushed the v13 branch 3 times, most recently from 45fc55e to 2a9a5b0 Compare June 15, 2020 16:24

Bibo-Joshi added 2 commits June 15, 2020 18:46

Temporarily enable tests for the v13 branch

8862a32

Merge branch 'v13' into refactor-job-queue

0e02483

# Conflicts: # telegram/ext/jobqueue.py

Bibo-Joshi force-pushed the v13 branch from 2a9a5b0 to 1fdc197 Compare June 16, 2020 15:08

Bibo-Joshi added 2 commits June 20, 2020 16:35

Increase coverage

e56af1c

Remove JobQueue.tick()

c723f5b

# Was intended for interal use anyways # Fixes tests

Poolitzer requested changes Jun 24, 2020

View reviewed changes

Bibo-Joshi added 2 commits June 24, 2020 20:51

Address review

6e1a014

Merge branch 'refactor-job-queue' of https://github.com/python-telegr…

8e587fb

…am-bot/python-telegram-bot into refactor-job-queue

Bibo-Joshi force-pushed the v13 branch from 1fdc197 to b3e9be1 Compare June 24, 2020 18:54

jh0ker reviewed Jun 24, 2020

View reviewed changes

tests/test_jobqueue.py Outdated Show resolved Hide resolved

jh0ker reviewed Jun 24, 2020

View reviewed changes

Temporarily enable tests for the v13 branch

b7e635b

Bibo-Joshi force-pushed the v13 branch from b3e9be1 to b7e635b Compare June 25, 2020 05:56

Bibo-Joshi added 2 commits June 25, 2020 19:23

Merge branch 'v13' into refactor-job-queue

585e5f2

Address review

9171103

Bibo-Joshi mentioned this pull request Jun 25, 2020

[BUG] Errors in Job callbacks are not passed to error handler #2004

Closed

Dispatch errors

8d22dcf

Bibo-Joshi added 2 commits July 2, 2020 14:24

Fix handling of job_kwargs

d44acb9

Merge branch 'refactor-job-queue' of github.com:/python-telegram-bot/…

271a105

…python-telegram-bot into refactor-job-queue

Bibo-Joshi removed the pending review label Jul 8, 2020

Bibo-Joshi added 2 commits July 8, 2020 22:17

Remove possibility to pass a Bot to JobQueue

46dcff4

Merge remote-tracking branch 'origin/refactor-job-queue' into refacto…

065ce12

…r-job-queue

Bibo-Joshi force-pushed the v13 branch from 20ee6e1 to 260b796 Compare July 10, 2020 10:59

Merge branch 'v13' into refactor-job-queue

8867e1c

Bibo-Joshi merged commit 088e608 into v13 Jul 10, 2020

Bibo-Joshi deleted the refactor-job-queue branch July 10, 2020 11:11

Bibo-Joshi mentioned this pull request Jul 18, 2020

Refactor JobQueue #1936

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor JobQueue #1981

Refactor JobQueue #1981

Bibo-Joshi commented Jun 5, 2020 •

edited

Loading

Poolitzer left a comment

Poolitzer Jun 24, 2020

Bibo-Joshi Jun 24, 2020

jh0ker Jun 24, 2020 •

edited

Loading

Bibo-Joshi Jun 25, 2020

Bibo-Joshi commented Jul 10, 2020

Refactor JobQueue #1981

Refactor JobQueue #1981

Conversation

Bibo-Joshi commented Jun 5, 2020 • edited Loading

Poolitzer left a comment

Choose a reason for hiding this comment

Poolitzer Jun 24, 2020

Choose a reason for hiding this comment

Bibo-Joshi Jun 24, 2020

Choose a reason for hiding this comment

jh0ker Jun 24, 2020 • edited Loading

Choose a reason for hiding this comment

Bibo-Joshi Jun 25, 2020

Choose a reason for hiding this comment

Bibo-Joshi commented Jul 10, 2020

Bibo-Joshi commented Jun 5, 2020 •

edited

Loading

jh0ker Jun 24, 2020 •

edited

Loading