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

Add support for concurrent requests via asyncio and aiohttp #207

Closed
wants to merge 11 commits into from
Closed

Add support for concurrent requests via asyncio and aiohttp #207

wants to merge 11 commits into from

Conversation

chrisimcevoy
Copy link

As suggested in #187

This is my first public PR, so please be gentle ;)

There are a significant amount of new classes here, because it's generally considered a bad idea to mix asynchronous and synchronous code. However, I've tried to keep the usage similar between the Client and the new AsyncClient where possible. I've also tried to inherit from existing classes to minimise impact, where it seemed logical to do so.

The major difference is that to instantiate the AsyncClient, you need to run AsyncClient.create() which is both a coroutine and a classmethod and returns the client instance. This my preferred way of working around the need to await coroutines in init() (see ).

Once your AsyncClient is instantiated, you can call client.service.operation() as normal, but this is now a coroutine which needs to be awaited. I've included an example showing usage (examples/async_example.py).

There are however a few caveats of which I'm aware:

  • I've used the async/await syntax which is only available from Python 3.5. This may not jive well with you as the rest of the project seems to strive for compatibility.
  • Owing to my relative lack of experience with pytest, I haven't written any tests for this. Invite others with more experience to do so...?

Converted SoapBinding's send() and process_reply() methods to
coroutines. Async versions of Soap11Binding/Soap12Binding inherit from
both AsyncSoapBinding and their synchronous counterparts.
Converted methods to coroutines where necessary. AsyncDefinition returns
AsyncSoapBindings - no AsyncHttp bindings yet.
Converted Transport methods to coroutines. Reading wsdl from local
filesystem might be improved using asyncio...?
Asyncio/Aiohttp-compatible Client. Use AsyncClient.create() coroutine to
instantiate.
Was getting an error as positional parameters were missing. Could this
be handled more elegantly?
@mvantellingen
Copy link
Owner

Hi @chrisimcevoy,

First of all, this looks really cool! Thanks. So the issue now is that this uses 3.5+ only syntax, so we need to see if can we can make this work as something like a plugin that can optionally be enabled. However to be honest, I first want to get a stable 1.0 release out before making these changes.

I will keep this PR and might build upon it for a later version.

Again really appreciated!

@mvantellingen mvantellingen added this to the future milestone Oct 15, 2016
@chrisimcevoy
Copy link
Author

Absolutely - I imagine there would need to be some sort of difficult decision made in that regard.

You could make this compatible with python 3.4 by instead coding the coroutines using the old yield from/@asyncio.coroutine decorator syntax, which should also run ok when called from async/await code in python 3.5 and later as the 3.4 syntax is forwards-compatible.

Also, the 3.4 asyncio was backported to 3.3, it's just not in the standard libs for that version.

I've seen trollius discussed on other packages where python 2.7 support is needed, but unfortunately it's no longer maintained. Sorry, I have no real expertise in that area as I pretty much exclusively work in 3.4 or later.

So, lots of different ways you could approach this down the line, each with their pros and cons. With that in mind, I completely understand your desire to reach 1.0 before dealing with all that!

mvantellingen added a commit that referenced this pull request Nov 6, 2016
This adds an optional AsyncTransport class which only works for
Python 3.5. Code is heavily inspired by the pull-request from
chrisimcevoy (see #207)
@mvantellingen
Copy link
Owner

Hi @chrisimcevoy, I've taken your code and incorporated in a way that makes is completely optional. See #251 .

Really cool!

mvantellingen added a commit that referenced this pull request Nov 13, 2016
This adds an optional AsyncTransport class which only works for
Python 3.5. Code is heavily inspired by the pull-request from
chrisimcevoy (see #207)
mvantellingen added a commit that referenced this pull request Nov 24, 2016
This adds an optional AsyncTransport class which only works for
Python 3.5. Code is heavily inspired by the pull-request from
chrisimcevoy (see #207)
mvantellingen added a commit that referenced this pull request Nov 24, 2016
This adds an optional AsyncTransport class which only works for
Python 3.5. Code is heavily inspired by the pull-request from
chrisimcevoy (see #207)
@mvantellingen
Copy link
Owner

Just merged the initial asyncio support. In the future I want to extend the support with the functionality in this PR. However I will be closing it for now, but this PR is referenced in the changelog!

Again many thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants