-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Builder pattern for ext #2646
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
Builder pattern for ext #2646
Conversation
…ryptionError (#2621) * move telegramdecryptionerror to error.py * Change error class name
* feat: add docs about docs * fix: improve looks * fix: make link work * fix: this looks better * Improved markdown, updated link * Less justifying Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
* Fix incomplete type annotations for CallbackContext Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
* Feat: Custom pytest marker Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
* Make basepersistence methods abstractmethod Signed-off-by: starry69 <starry369126@outlook.com> Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
# Conflicts: # telegram/ext/conversationhandler.py # telegram/ext/dispatcher.py # telegram/ext/updater.py # tests/conftest.py # tests/test_dispatcher.py # tests/test_jobqueue.py # tests/test_persistence.py # tests/test_updater.py
# Conflicts: # telegram/ext/__init__.py # telegram/ext/dispatcher.py # telegram/utils/promise.py
# Conflicts: # telegram/ext/__init__.py # telegram/ext/callbackcontext.py # telegram/ext/conversationhandler.py # telegram/ext/dispatcher.py # telegram/ext/jobqueue.py # telegram/ext/updater.py # tests/conftest.py # tests/test_dispatcher.py # tests/test_jobqueue.py # tests/test_persistence.py # tests/test_updater.py
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
# Conflicts: # telegram/ext/jobqueue.py
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
# Conflicts: # examples/deeplinking.py # examples/inlinebot.py # telegram/ext/dispatcher.py # telegram/ext/updater.py # tests/conftest.py # tests/test_dispatcher.py # tests/test_updater.py
# Conflicts: # telegram/bot.py # telegram/ext/__init__.py # telegram/ext/callbackcontext.py # telegram/ext/conversationhandler.py # telegram/ext/dispatcher.py # telegram/ext/extbot.py # telegram/ext/jobqueue.py # telegram/ext/updater.py # tests/conftest.py # tests/test_bot.py # tests/test_dispatcher.py # tests/test_jobqueue.py # tests/test_persistence.py # tests/test_stack.py # tests/test_updater.py
# Conflicts: # examples/passportbot.py # examples/persistentconversationbot.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left 2 minor comments.
PR approved. Merge at will.
""" | ||
return self._set_request(request) | ||
|
||
def private_key(self: BuilderType, private_key: bytes, password: bytes = None) -> BuilderType: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that a really nice touch would be to have private_key accept either bytes
(the key itself) or Path|str
and then we'll open the file and read the file for the user.
Anyway, lets merge this PR already, and if you like the idea about the private_key, lets do it in another PR.
# Conflicts: # telegram/ext/updater.py
Closes #2556
Notes:
ExtBotBuilder
class. Currently only therequest
argument ofExtBot
would profit from with. Then again, the init ofExtBot
may become more complicated in the future if we add more features. If we want to add a corresponding class, we need to think about if we still want to allow for manual instantiation ofExtBot
or not …Updater
can be used withoutDispatcher
. IMO this emphasizes that both have different tasks and for advanced users it may be helpful to outsource the fetching of updates while doing the processing manually.Dispatcher/Updater.exception_event
properly documented attributes and they are no longer protected. Whileexception_event
was probably initially meant as means for private communication betweenUpdater
andDispatcher
I do feel like it could be useful to have it more publically available for advaced, custom setups.DefaultContextType = CallbackContext[ExtBot, Dict, Dict, Dict]
. This made sence for the type hinting in the builders, but it seemed clever to me to make this also publically available to allow users to improve their type hints.DefaultContextType
is the correct annotation if you're not using a customContextTypes
object. The import ofDefaultContextType
can be fine tuned in the context of Clear up imports policy #2468builder.bot(bot)
would fail because you already set a token. I quickly gave up on this because it became too excessive. Instead type hinting "only" coversjob_queue
,bot
,persistence
,context_types
includingbot/chat/user_data
. This essentially makes sure that one can get proper type hints for all the attributes of thecontext
argument. E.g. now it tells you ifcontext.bot
istg.Bot
orExtBot
and also ifcontext.job_queue
isNone
set_dispatcher/updater_class
don't have proper type hints, i.e. you won't know ifupdater
orupdater/context.dispatcher
are instances of subclasses. IISC that's near impossible to get right and users that are so advanced that they want custom subclasses, should be able to handle that …TODO:
Updare/Dispatcher.builder() -> Updater/DispatcherBuilder()
and use those in the examplestest_dispatcher/test_less_than_one_worker_warning
Checklist for PRs
.. versionadded:: version
,.. versionchanged:: version
or.. deprecated:: version
to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)