From aa108142b94c2356a001a22890fff64ace7976eb Mon Sep 17 00:00:00 2001 From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com> Date: Sun, 3 Oct 2021 12:55:51 +0200 Subject: [PATCH 1/3] Make Jobs callable and pass them to error handlers --- docs/source/telegram.ext.job.rst | 1 + telegram/ext/callbackcontext.py | 12 +++-- telegram/ext/dispatcher.py | 14 ++++-- telegram/ext/jobqueue.py | 85 +++++++++++++++----------------- tests/test_jobqueue.py | 8 +-- 5 files changed, 66 insertions(+), 54 deletions(-) diff --git a/docs/source/telegram.ext.job.rst b/docs/source/telegram.ext.job.rst index 50bfd9e7b6b..d6c4f69146c 100644 --- a/docs/source/telegram.ext.job.rst +++ b/docs/source/telegram.ext.job.rst @@ -6,3 +6,4 @@ telegram.ext.Job .. autoclass:: telegram.ext.Job :members: :show-inheritance: + :special-members: __call__ diff --git a/telegram/ext/callbackcontext.py b/telegram/ext/callbackcontext.py index e7edc4b5aaa..fe48068ec4e 100644 --- a/telegram/ext/callbackcontext.py +++ b/telegram/ext/callbackcontext.py @@ -86,7 +86,8 @@ class CallbackContext(Generic[UD, CD, BD]): that raised the error. Only present when the raising function was run asynchronously using :meth:`telegram.ext.Dispatcher.run_async`. job (:class:`telegram.ext.Job`): Optional. The job which originated this callback. - Only present when passed to the callback of :class:`telegram.ext.Job`. + Only present when passed to the callback of :class:`telegram.ext.Job` or in error + handlers if the error is caused by a job. """ @@ -231,6 +232,7 @@ def from_error( dispatcher: 'Dispatcher[CCT, UD, CD, BD]', async_args: Union[List, Tuple] = None, async_kwargs: Dict[str, object] = None, + job: 'Job' = None, ) -> 'CCT': """ Constructs an instance of :class:`telegram.ext.CallbackContext` to be passed to the error @@ -244,12 +246,15 @@ def from_error( error (:obj:`Exception`): The error. dispatcher (:class:`telegram.ext.Dispatcher`): The dispatcher associated with this context. - async_args (List[:obj:`object`]): Optional. Positional arguments of the function that + async_args (List[:obj:`object`], optional): Positional arguments of the function that raised the error. Pass only when the raising function was run asynchronously using :meth:`telegram.ext.Dispatcher.run_async`. - async_kwargs (Dict[:obj:`str`, :obj:`object`]): Optional. Keyword arguments of the + async_kwargs (Dict[:obj:`str`, :obj:`object`], optional): Keyword arguments of the function that raised the error. Pass only when the raising function was run asynchronously using :meth:`telegram.ext.Dispatcher.run_async`. + job (:class:`telegram.ext.Job`, optional): The job associated with the error. + + .. versionadded:: 14.0 Returns: :class:`telegram.ext.CallbackContext` @@ -258,6 +263,7 @@ def from_error( self.error = error self.async_args = async_args self.async_kwargs = async_kwargs + self.job = job return self @classmethod diff --git a/telegram/ext/dispatcher.py b/telegram/ext/dispatcher.py index cf60d8d6ad0..59fd11bcb3e 100644 --- a/telegram/ext/dispatcher.py +++ b/telegram/ext/dispatcher.py @@ -53,8 +53,7 @@ if TYPE_CHECKING: from telegram import Bot - from telegram.ext import JobQueue - from telegram.ext.callbackcontext import CallbackContext + from telegram.ext import JobQueue, Job, CallbackContext DEFAULT_GROUP: int = 0 @@ -668,6 +667,7 @@ def dispatch_error( update: Optional[object], error: Exception, promise: Promise = None, + job: 'Job' = None, ) -> bool: """Dispatches an error by passing it to all error handlers registered with :meth:`add_error_handler`. If one of the error handlers raises @@ -686,6 +686,9 @@ def dispatch_error( error (:obj:`Exception`): The error that was raised. promise (:class:`telegram.utils.Promise`, optional): The promise whose pooled function raised the error. + job (:class:`telegram.ext.Job`, optional): The job that caused the error. + + .. versionadded:: 14.0 Returns: :obj:`bool`: :obj:`True` if one of the error handlers raised @@ -697,7 +700,12 @@ def dispatch_error( if self.error_handlers: for callback, run_async in self.error_handlers.items(): # pylint: disable=W0621 context = self.context_types.context.from_error( - update, error, self, async_args=async_args, async_kwargs=async_kwargs + update=update, + error=error, + dispatcher=self, + async_args=async_args, + async_kwargs=async_kwargs, + job=job, ) if run_async: self.run_async(callback, update, context, update=update) diff --git a/telegram/ext/jobqueue.py b/telegram/ext/jobqueue.py index 208481e6dbd..778c07760a0 100644 --- a/telegram/ext/jobqueue.py +++ b/telegram/ext/jobqueue.py @@ -19,19 +19,16 @@ """This module contains the classes JobQueue and Job.""" import datetime -import logging -from typing import TYPE_CHECKING, Callable, List, Optional, Tuple, Union, cast, overload +from typing import TYPE_CHECKING, Callable, Optional, Tuple, Union, cast, overload import pytz -from apscheduler.events import EVENT_JOB_ERROR, EVENT_JOB_EXECUTED, JobEvent from apscheduler.schedulers.background import BackgroundScheduler from apscheduler.job import Job as APSJob -from telegram.ext.callbackcontext import CallbackContext from telegram.utils.types import JSONDict if TYPE_CHECKING: - from telegram.ext import Dispatcher + from telegram.ext import Dispatcher, CallbackContext import apscheduler.job # noqa: F401 @@ -44,35 +41,15 @@ class JobQueue: """ - __slots__ = ('_dispatcher', 'logger', 'scheduler') + __slots__ = ('_dispatcher', 'scheduler') def __init__(self) -> None: self._dispatcher: 'Dispatcher' = None # type: ignore[assignment] - self.logger = logging.getLogger(self.__class__.__name__) self.scheduler = BackgroundScheduler(timezone=pytz.utc) - self.scheduler.add_listener( - self._update_persistence, mask=EVENT_JOB_EXECUTED | EVENT_JOB_ERROR - ) - - # Dispatch errors and don't log them in the APS logger - def aps_log_filter(record): # type: ignore - return 'raised an exception' not in record.msg - - logging.getLogger('apscheduler.executors.default').addFilter(aps_log_filter) - self.scheduler.add_listener(self._dispatch_error, EVENT_JOB_ERROR) - - def _build_args(self, job: 'Job') -> List[CallbackContext]: - return [self._dispatcher.context_types.context.from_job(job, self._dispatcher)] def _tz_now(self) -> datetime.datetime: return datetime.datetime.now(self.scheduler.timezone) - def _update_persistence(self, _: JobEvent) -> None: - self._dispatcher.update_persistence() - - def _dispatch_error(self, event: JobEvent) -> None: - self._dispatcher.dispatch_error(None, event.exception) - @overload def _parse_time_input(self, time: None, shift_day: bool = False) -> None: ... @@ -169,11 +146,11 @@ def run_once( date_time = self._parse_time_input(when, shift_day=True) j = self.scheduler.add_job( - callback, + job, name=name, trigger='date', run_date=date_time, - args=self._build_args(job), + args=(self._dispatcher,), timezone=date_time.tzinfo or self.scheduler.timezone, **job_kwargs, ) @@ -261,9 +238,9 @@ def run_repeating( interval = interval.total_seconds() j = self.scheduler.add_job( - callback, + job, trigger='interval', - args=self._build_args(job), + args=(self._dispatcher,), start_date=dt_first, end_date=dt_last, seconds=interval, @@ -317,9 +294,9 @@ def run_monthly( job = Job(callback, context, name, self) j = self.scheduler.add_job( - callback, + job, trigger='cron', - args=self._build_args(job), + args=(self._dispatcher,), name=name, day='last' if day == -1 else day, hour=when.hour, @@ -374,9 +351,9 @@ def run_daily( job = Job(callback, context, name, self) j = self.scheduler.add_job( - callback, + job, name=name, - args=self._build_args(job), + args=(self._dispatcher,), trigger='cron', day_of_week=','.join([str(d) for d in days]), hour=time.hour, @@ -416,7 +393,7 @@ def run_custom( name = name or callback.__name__ job = Job(callback, context, name, self) - j = self.scheduler.add_job(callback, args=self._build_args(job), name=name, **job_kwargs) + j = self.scheduler.add_job(job, args=(self._dispatcher,), name=name, **job_kwargs) job.job = j return job @@ -434,7 +411,7 @@ def stop(self) -> None: def jobs(self) -> Tuple['Job', ...]: """Returns a tuple of all *scheduled* jobs that are currently in the ``JobQueue``.""" return tuple( - Job._from_aps_job(job, self) # pylint: disable=W0212 + Job._from_aps_job(job) # pylint: disable=protected-access for job in self.scheduler.get_jobs() ) @@ -509,11 +486,34 @@ def __init__( self.job = cast(APSJob, job) # skipcq: PTC-W0052 def run(self, dispatcher: 'Dispatcher') -> None: - """Executes the callback function independently of the jobs schedule.""" + """Executes the callback function independently of the jobs schedule. Also calls + :meth:`telegram.ext.Dispatcher.update_persistence` + + Args: + dispatcher (:class:`telegram.ext.Dispatcher`): The dispatcher this job is associated + with. + """ try: self.callback(dispatcher.context_types.context.from_job(self, dispatcher)) except Exception as exc: - dispatcher.dispatch_error(None, exc) + dispatcher.dispatch_error(None, exc, job=self) + finally: + dispatcher.update_persistence(None) + + def __call__(self, dispatcher: 'Dispatcher') -> None: + """Shortcut for:: + + job.run(dispatcher) + + Warning: + The fact that jobs are callable should be considered an implementation detail and not + as part of PTBs public API. + + Args: + dispatcher (:class:`telegram.ext.Dispatcher`): The dispatcher this job is associated + with. + """ + self.run(dispatcher=dispatcher) def schedule_removal(self) -> None: """ @@ -551,13 +551,8 @@ def next_t(self) -> Optional[datetime.datetime]: return self.job.next_run_time @classmethod - def _from_aps_job(cls, job: APSJob, job_queue: JobQueue) -> 'Job': - # context based callbacks - if len(job.args) == 1: - context = job.args[0].job.context - else: - context = job.args[1].context - return cls(job.func, context=context, name=job.name, job_queue=job_queue, job=job) + def _from_aps_job(cls, job: APSJob) -> 'Job': + return job.func def __getattr__(self, item: str) -> object: return getattr(self.job, item) diff --git a/tests/test_jobqueue.py b/tests/test_jobqueue.py index cfeb94a30b0..2ea6271c16e 100644 --- a/tests/test_jobqueue.py +++ b/tests/test_jobqueue.py @@ -96,7 +96,7 @@ def job_context_based_callback(self, context): self.result += 1 def error_handler_context(self, update, context): - self.received_error = str(context.error) + self.received_error = (str(context.error), context.job) def error_handler_raise_error(self, *args): raise Exception('Failing bigly') @@ -426,10 +426,12 @@ def test_dispatch_error_context(self, job_queue, dp): job = job_queue.run_once(self.job_with_exception, 0.05) sleep(0.1) - assert self.received_error == 'Test Error' + assert self.received_error[0] == 'Test Error' + assert self.received_error[1] is job self.received_error = None job.run(dp) - assert self.received_error == 'Test Error' + assert self.received_error[0] == 'Test Error' + assert self.received_error[1] is job # Remove handler dp.remove_error_handler(self.error_handler_context) From 5401e76121f79a6c8572204e7230abdd2f0f4525 Mon Sep 17 00:00:00 2001 From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com> Date: Sun, 3 Oct 2021 12:59:37 +0200 Subject: [PATCH 2/3] More versioning directives --- telegram/ext/jobqueue.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/telegram/ext/jobqueue.py b/telegram/ext/jobqueue.py index 778c07760a0..ba44780a137 100644 --- a/telegram/ext/jobqueue.py +++ b/telegram/ext/jobqueue.py @@ -487,7 +487,10 @@ def __init__( def run(self, dispatcher: 'Dispatcher') -> None: """Executes the callback function independently of the jobs schedule. Also calls - :meth:`telegram.ext.Dispatcher.update_persistence` + :meth:`telegram.ext.Dispatcher.update_persistence`. + + .. versionchaged:: 14.0 + Calls :meth:`telegram.ext.Dispatcher.update_persistence`. Args: dispatcher (:class:`telegram.ext.Dispatcher`): The dispatcher this job is associated @@ -509,6 +512,8 @@ def __call__(self, dispatcher: 'Dispatcher') -> None: The fact that jobs are callable should be considered an implementation detail and not as part of PTBs public API. + .. versionadded:: 14.0 + Args: dispatcher (:class:`telegram.ext.Dispatcher`): The dispatcher this job is associated with. From f48d083520b99f7b2d7ccda321a84c23e0ee929c Mon Sep 17 00:00:00 2001 From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com> Date: Sun, 3 Oct 2021 18:03:32 +0200 Subject: [PATCH 3/3] One more versioning directive --- telegram/ext/callbackcontext.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/telegram/ext/callbackcontext.py b/telegram/ext/callbackcontext.py index fe48068ec4e..d89fe5cce0d 100644 --- a/telegram/ext/callbackcontext.py +++ b/telegram/ext/callbackcontext.py @@ -89,6 +89,9 @@ class CallbackContext(Generic[UD, CD, BD]): Only present when passed to the callback of :class:`telegram.ext.Job` or in error handlers if the error is caused by a job. + .. versionchanged:: 14.0 + :attr:`job` is now also present in error handlers if the error is caused by a job. + """ __slots__ = (