Skip to content
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

Asyncio example is completely wrong, update #988

Open
agronick opened this issue Apr 26, 2021 · 5 comments
Open

Asyncio example is completely wrong, update #988

agronick opened this issue Apr 26, 2021 · 5 comments
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap

Comments

@agronick
Copy link

Issue Summary

I'm talking about this example https://github.com/sendgrid/sendgrid-python/blob/main/use_cases/asynchronous_mail_send.md

Steps to Reproduce

Using asyncio is more than just putting async in front of the function doing the http request. In fact this is the completely wrong way to use asyncio and will actually destroys performance as this blocks the event loop from doing any other work while the http request is being made.

To use asyncio correctly the tcp socket sending the data must be using asyncio and one of the low level asyncio socket functions. https://docs.python.org/3/library/asyncio-stream.html These are awaited, allowing the event loop to do other work.

What you have suggested is EXTREMELY bad practice and will stall the servers of anyone who uses it. This is why you can't use requests or urllib with asyncio. You have to use something like aiohttp.

The right way to use asyncio with your library is to run it in a thread pool executor so that the blocking IO stays off the main thread.

You would be better off deleting that example than keeping it in it's current form.

@thinkingserious thinkingserious changed the title Asyncio example is completely wrong Asyncio example is completely wrong, update Apr 27, 2021
@thinkingserious
Copy link
Contributor

Hello @agronick,

Thank you for raising this issue!

This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.

Here are some related issues and PRs to consider upon update:

With best regards,

Elmer

@thinkingserious thinkingserious added status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap labels Apr 27, 2021
@agronick
Copy link
Author

#953 is exactly what I'm talking about but it was closed.

@parikls
Copy link

parikls commented Sep 24, 2021

upvote. unfortunately current SDK is not compatible with asyncio which makes it unusable with such things like aiohttp or fastapi

@dacevedo12
Copy link

dacevedo12 commented May 30, 2024

Hi, it's 2024 and the example is still wrong. Open to PRs? I can help

It's as simple as wrapping the call using asyncio.to_thread so it doesn't block the main thread.

Ideally though the SDK itself would provide support for asyncio, maybe through a transport layer that uses aiohttp or httpx

@urbanonymous
Copy link

I'll just use the api lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
Development

No branches or pull requests

5 participants