8000 Asyncio by Bibo-Joshi · Pull Request #2731 · python-telegram-bot/python-telegram-bot · GitHub
[go: up one dir, main page]
Skip to content

Asyncio #2731

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 171 commits into from
Apr 24, 2022
Merged

Asyncio #2731

merged 171 commits into from
Apr 24, 2022

Conversation

Bibo-Joshi
Copy link
Member
@Bibo-Joshi Bibo-Joshi commented Oct 14, 2021 •
edited by harshil21
Loading

Supersedes #2402. Closes #2288 when ready - which is not in the forseable future :D

  • ext-tests are deleted for now so that we can first concentrate on getting telegram to work
  • any changes to v14 should be merged into this ASAP

TODO

Let's use this as place to write everything down that comes to mind while working on this - will be a lot

  • move PtbRequestBase and HttpxRequest to a tg.request module/package. Maybe rename to RequestBase and HTTPXRequest
  • Improve handling of files in request - probably move that logic into a standalone method or even into Bot?
  • Update error code handling in request, use the enum from the std-lib
  • Check if we get coroutine was never awaited somewhere in the tests and also check if we can make those warnings fail the tests
  • check back with noam on whether the timeout parameter should be used for write_timeout as well in in BaseRequest
  • Check back with Noam about httpx connection pool limits: IISC on pool_timeout we want to create a new connection instead of dropping the request?!
  • Think about how we handle non-async callbacks for handlers and jobs. We might want to make sure that they run in a background thread - which is the default behavior of APscheduler

  • Think about what to do with exceptions in the long running tasks of updater. Currently we put them in the update_queue, which isn't really nice
  • persistence
  • think about a replacement for exception_event
  • a lot of tests
    • For JobQueue test that weekdays are working as expected to avoid regression when we update APS - see here and here
  • check for added # TODO comments
  • update examples. for chatmemberbot.py we can also include the changes proposed here

Things that don't need to be done in this PR

If they are not, create issues for them!

  • set socketops
  • switch to a different library for webhooks - maybe starlette (backend of fastAPI)?
  • Add transaction support to BaspePersistence, i.e. abstract methods start/end_transaction so that we can do
  1. tell persistence that we're going to start transaction
  2. hand over data
  3. tell persistence that I'm finished

Review

Thinks to look out for in a review

  • double check if we need to update docs regarding accepted types
  • Check for unwanted changes (also in tests!) - these are likely to be there because merging v14 was a mess
    • Some test_slots_behavior tests went missing for some reason. check the diff and re-add them
  • Add .. versionadded:: version, .. versionchanged:: version or .. deprecated:: version to the docstrings
  • Also manually check whether thumbs are properly handled everywhere(!) where you can pass a thumb
  • take all the examples up for a test run
  • manually test webhook setups - this PR changes if the webserver listens to https://… or http://…
  • manually test how kill pid works when using App.run_* & starting the script as a background task

???

I wrote this down while working on a signal-handler based exit. not sure, if still relevant

@Bibo-Joshi Bibo-Joshi added enhancement 🛠 breaking change type: breaking labels Oct 14, 2021
@Bibo-Joshi Bibo-Joshi added this to the v14 milestone Oct 14, 2021
github-actions[bot]
github-actions bot reviewed Oct 14, 2021
View reviewed changes
Copy link
@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Looks like you edited README.rst or README_RAW.rst. I'm just a friendly reminder to apply relevant changes to both of those files :)

github-actions[bot]
github-actions bot reviewed Oct 14, 2021
View reviewed changes
Copy link
@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there. Relax, I am just a little warning for the maintainers to release directly after merging your PR, otherwise we have broken examples and people might get confused :)

github-actions[bot]
github-actions bot reviewed Oct 14, 2021
View reviewed changes
Copy link
@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Looks like you edited the (dev) requirements or the pre-commit hooks. I'm just a friendly reminder to keep the pre-commit hook versions in sync with the dev requirements and the additional dependencies for the hooks in sync with the requirements :)

@Bibo-Joshi Bibo-Joshi mentioned this pull request Oct 25, 2021
Closed
4 tasks
@Bibo-Joshi Bibo-Joshi added the 🛠 refactor change type: refactor label Nov 2, 2021
@Bibo-Joshi Bibo-Joshi force-pushed the v14 branch 3 times, most recently from 5c26c91 to 74c4270 Compare December 13, 2021 18:14
harshil21
harshil21 reviewed Jan 20, 2022
View reviewed changes
@Bibo-Joshi Bibo-Joshi changed the title Not even WIP: Asyncio WIP: Asyncio Feb 8, 2022
@Bibo-Joshi
Merge branch 'v14' into asyncio
8000
c8b114f 
# Conflicts:
#	telegram/_files/file.py
#	telegram/ext/_basepersistence.py
#	telegram/ext/_builders.py
#	telegram/ext/_callbackqueryhandler.py
#	telegram/ext/_chatjoinrequesthandler.py
#	telegram/ext/_chatmemberhandler.py
#	telegram/ext/_choseninlineresulthandler.py
#	telegram/ext/_commandhandler.py
#	telegram/ext/_dispatcher.py
#	telegram/ext/_handler.py
#	telegram/ext/_inlinequeryhandler.py
#	telegram/ext/_messagehandler.py
#	telegram/ext/_picklepersistence.py
#	telegram/ext/_pollanswerhandler.py
#	telegram/ext/_pollhandler.py
#	telegram/ext/_precheckoutqueryhandler.py
#	telegram/ext/_shippingqueryhandler.py
#	telegram/ext/_stringcommandhandler.py
#	telegram/ext/_stringregexhandler.py
#	telegram/ext/_typehandler.py
#	telegram/ext/_updater.py
#	telegram/request.py
@Bibo-Joshi
Start fixing some sphinx warnings
362e7b2
@Bibo-Joshi
More reference fixes
ecd909d
@Bibo-Joshi
Try fixing wrong source links on RTD
d0565fc
@Bibo-Joshi
Slowly get started on request tests
985d24f
harshil21
harshil21 reviewed Apr 20, 2022
View reviewed changes
harshil21
harshil21 reviewed Apr 22, 2022
View reviewed changes
Copy link
Member
@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uploading files rework review.

Btw codecov shows that quite a few lines in Application and ConversationHandler are not covered in tests?

@Bibo-Joshi Bibo-Joshi merged commit ad5a1fb into v14 Apr 24, 2022
@Bibo-Joshi Bibo-Joshi deleted the asyncio branch April 24, 2022 11:19
@github-actions github-actions bot locked and limited conversation to collaborators May 1, 2022
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@harshil21 harshil21 harshil21 left review comments

@github-actions github-actions[bot] github-actions[bot] left review comments

Assignees
No one assigned
Labels
🛠 breaking change type: breaking 🔌 enhancement pr description: enhancement 🛠 refactor change type: refactor
Projects
None yet
Milestone
v20.0a0
Development

Successfully merging this pull request may close these issues.
3 participants
@Bibo-Joshi @harshil21 @Poolitzer
0