-
Notifications
You must be signed in to change notification settings - Fork 6k
Description
TL;DR:
Known bugs are
- Defaults not working [BUG] Defaults not working with MessageQueue #2015
- Hanging when using ctrl + c [BUG] Bot hangs when using ctrl + c and MessageQueue #2012
- Context manager for opening file broken [BUG] Context manager not working with MessageQueue #2035
- Shortcuts lost their *args and **kwargs [BUG] Shortcuts lost their *args and **kwargs, resulting messagequeue isgroup= and other broke #2392
- Errors are being hidden [BUG] Promise ignores exceptions #3201
Long version:
Using MessageQueue currently requires quite some manual work and there also the aformentioned bugs. Time to live up to the wikis statement
We plan to tightly couple it with telegram.Bot so that it could be used more conveniently (and even implicitly unless other specified).
So here is what my idea for a refactoring is. I'd be happy for feedback before getting started :)
Refactoring of MessageQueue and DelayQueue
DQ.exc_route: I'd like to rename toerror_handlerin accordance withDispatcher.add_error_handler.- If a
Dispatcheris used, it would make sense to have the execution of the promises to behave like the refactoredDipsatcher._pooled, which also includes proper error handling. That can be easily achieved by passing theDispatcherto either theBotor theMessageQueue. Not sure yet, what makes more sense … - I think I'd like to repace the
__call__methods withprocessor whatever. That's more explicit and easier to read in the code. - Add
MQ.add/remove_delay_queueso that users can add custom delay queues in addition to the default & the group one. One should be able to specify a parent (defaulting to the default DQ) fornew_dq, such that everything going throguhnew_dqwill also go through the parent.MQ.add_remove_delay_queuesohuld also start/stop theDQin case we're adding them at runtime. Edit: This will in fact close Multiple message queues for a bot #1369 , as I just notice - Add constants for the default & the group DQ
- Handle the stopping of
MQcorrectly, some hints are already given in [BUG] Bot hangs when using ctrl + c and MessageQueue #2012
Integration into Bot
Add a MessageQueue to Bot by default and rebuild all methods in the fashion
def badly_sketched_messagequeue_wrapper(func, *args, **kwargs):
delay_queue = kwargs.pop('delay_queue')
if delay_queue == self.message_queue.DEFAULT_DQ and int(kwargs.get('chat_id')) < 0:
return self.message_queue.process(func, self.message_queue.GROUP_DQ)
elif delay_queue:
return self.message_queue.process(func, delay_queue)
return func(*args, **kwargs)
@badly_sketched_messagequeue_wrapper
def send_message(…, delay_queue=None):
...such that delay_queue allows to select the delay queue the method is passed through, replacing the current queue and isgroup kwargs
- Together with the changes to
Bot.__new__as in discussed [BUG] Defaults not working with MessageQueue #2015 , this should resolve [BUG] Defaults not working with MessageQueue #2015 - for cases where one only needs
Bot, withoutDispatcherorUpdater, one could do something like
def badly_sketched_messagequeue_wrapper(func, *args, **kwargs):
if self.message_queue.is_running:
# see above
return func(*args, **kwargs)
so users can easily opt-into MessageQueue with bot.mq.start/stop()
Integration with Defaults
Add Defaults.delay_queue defaulting to None (I'm not in favor of implicitly always using the MessageQueue)
Handling context managers
I think the approach detailed in #2035 is feasible (build InputFiles already in Promise.__init__). I can't think of any use case, where a context manager would be used for anything else that we'd to handle …
Edit: Now I can. See #2160
Backwards compatibility
All of above changes should be doable without breaking changes.
Hower, given that MessageQueue is essentially still in the same quite rough state as when introduced in #537 3.5 years ago, IMHO it would be okay to removethe current usage pattern (i.e. the @queuedmessage decorator) in the next major release (which probably will be mostly a "delete all the stuff" release anyway …)