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-based Python Client #5939

Closed
toumorokoshi opened this issue Jun 28, 2017 · 16 comments
Closed

Asyncio-based Python Client #5939

toumorokoshi opened this issue Jun 28, 2017 · 16 comments

Comments

@toumorokoshi
Copy link
Contributor

Description

I'm interested in adding code into the existing python client to a native asyncio client for swagger.

https://docs.python.org/3/library/asyncio.html

Does this already exist? Or should I talk to anyone before attempting this?

My General approach was going to be:

  1. create an ApiClientBase class to share common components between async and sync
  2. modify ApiClient to use ApiClientBase
  3. create an AsyncApiClient that uses asynchronous await / async syntax.

@masterandrey I see you're using async with the existing swagger client: do you have any comments?

Command line used for generation

-l python

@andgineer
Copy link

No, I could not use swagger client in async mode.

@wing328
Copy link
Contributor

wing328 commented Jun 28, 2017

Have you tried making the async call by providing the callback (optional) parameter? e.g. https://github.com/swagger-api/swagger-codegen/blob/master/samples/client/petstore/python/petstore_api/apis/pet_api.py#L159

@toumorokoshi
Copy link
Contributor Author

@wing328 that does indeed spawn a thread and call a callback, which is a form of asynchronous programming, but I was referring to the python3 asyncio. It is possible to integrate synchronous http libraries into asyncio via the future module (https://pymotw.com/3/asyncio/executors.html), but it still requires native threads to execute, which is undesirable due to limitations of system-thread based programming which asyncio is designed to overcome in the first place.

It sounds like it's worth giving the implementation a shot. thanks!

@wing328
Copy link
Contributor

wing328 commented Jun 30, 2017

@toumorokoshi thanks for providing more info.

Shall we use a HTTP lib that supports Python3 asyncio? What about aiohttp? I've not tried it myself but based on the example it looks like it may be suitable for our use case.

Other languages such as Java supports more than 1 HTTP library via a library option. This is possible by making "ApiClient" swappable with other implementation using different HTTP libraries (e.g. Retrofit2, Jersey 2.x, etc) Can we do something similar so that Python can still use the current API client implementation (which is compatible with Python 2.x) if they want to?

cc @taxpon @frol @scottrw93

@frol
Copy link
Contributor

frol commented Jun 30, 2017

asyncio (and aiohttp in particular) would be a neat feature to have! I would, however, not bother with Python <3.5 support. If one needs the older Python versions support, there will be a "classic" [synchronous] generator mode, thus the clients provider would publish two clients (client-classic, client-asyncio).

@wing328
Copy link
Contributor

wing328 commented Jun 30, 2017

Just to share a bit more. For JS, we recently introduced the "useES6" option to allow JS generator output codes that are no longer compatible with ES5.

For Python Flask generator, there's the option "supportPython2" to make the output compatible with Python 2.x

For asyncio, we can introduce similar option. (e.g. supportPython2 with default value "true". When set to false, it will generate code that is compatible with Python 3.4+ and leverages asyncio )

@frol
Copy link
Contributor

frol commented Jun 30, 2017

Well, asyncio is still not as widely applied to make it the default for Python 3.5. Synchronous APIs and even APIs with callbacks are still easier to understand and implement than asyncio in Python. If your project is not written in asyncio, than the API calls would look like this:

users_api = my_client.UsersApi()

import asyncio
loop = asyncio.get_event_loop()

users = loop.run_until_complete(users_api.get_users())

@toumorokoshi
Copy link
Contributor Author

I'd like to clarify my approach, because I think these are all valid concerns.

I'd like to create a new module for async based apis:

# alternatively from petstore_api.api.async import AsyncUsersApi
from petstore_api.async_api import AsyncUsersApi

users_api = AsyncUsersApi()

async def run_api_code():
    users = await user_api.get_users()

import asyncio
loop = asyncio.get_event_loop()

users = loop.run_until_complete(run_api_code())
  • This module will only work for Python3.4+ (async was introduced in 3.4 with older syntax that I can use)
  • It does not modify the old API code, instead sharing components and create a separate AsyncClient and api class.
  • no breaking changes, only additions.
  • it will use aiohttp under the hood.

I'm have a branch I'm working on now, with my implementation. Going a little slowly due to time, but it's not terribly difficult work:

https://github.com/toumorokoshi/swagger-codegen

Details:

@wing328 since we don't execute modules in python unless we load them, I can just write a python3 specific module that will raise an exception when it gets loaded by python 2. I'm hoping to write a 2-3 compatible library to make it easier for the client author.

Due to the requirement that asyncio uses coroutines throughout the whole call chain, I can't do something that is arguably better like:

user_api = UserApi(api_client=AsyncApiClient())

# this isn't marked as a coroutine in the UserApi class, so it will fail
await user_api.get_users()

I wish Python had designed it that way, it makes a lot of code way easier to write.

@wing328
Copy link
Contributor

wing328 commented Jun 30, 2017

I'm have a branch I'm working on now, with my implementation. Going a little slowly due to time, but it's not terribly difficult work:

@toumorokoshi I would suggest you to open a PR with the enhancement so that others can more easily review and test the change.

@wing328
Copy link
Contributor

wing328 commented Jun 30, 2017

@toumorokoshi btw, not sure if you've noticed. The Python API client in the 2.3.0 branch has been refactored (e.g. removed singleton) so I would suggest you to take a look and base the enhancements on that.

@frol
Copy link
Contributor

frol commented Jun 30, 2017

@toumorokoshi I don't see much point in having 2 copies of the client generated into a single module. Also, you will end up with a copy of the templates with just async prefix and a new ApiClient. I think, it is much better to just have a Codegen flag to generate either sync or async code, and that would be much easier to maintain and understand (for the end user). Python 3.4 support is also something that isn't worth a trouble as I haven't seen any problems in migrating up to 3.6, so please, use the new syntax.

@wing328 wing328 modified the milestones: v2.3.0, v2.2.3, Future Jul 2, 2017
@toumorokoshi
Copy link
Contributor Author

@wing328 sure, I'll start one up when I have a working sketch.

@frol I was thinking from the perspective of someone publishing the library, it might nice to have them in the same codebase, similar to Motor's support of Tornado and Asyncio:

https://github.com/mongodb/motor.

But, it's not a big deal either way. I'll can add an option for asyncio.

@frol
Copy link
Contributor

frol commented Jul 4, 2017

@toumorokoshi My thinking targets the consumers. I haven't seen a case where the need to use a module via different APIs in a single project would make sense. Given this fact, having two sets of APIs in a single client just bloats the installation.

@wing328
Copy link
Contributor

wing328 commented Jul 5, 2017 via email

@toumorokoshi
Copy link
Contributor Author

sounds good. I'll do a PR when I have a feature flag version ready.

@frol
Copy link
Contributor

frol commented Jul 10, 2017

@toumorokoshi

Due to the requirement that asyncio uses coroutines throughout the whole call chain, I can't do something that is arguably better like:

Since I haven't had my hands on AsyncIO, I believed you, but it seems that this statement is false, at least in Python 3.6:

import asyncio

async def qq(a):
    await asyncio.sleep(2)
    return 1

def qq2():
    return qq(1)

async def qq3():
    return await qq2()

l = asyncio.get_event_loop()
l.run_until_complete(qq3())

This code works just fine for me. Notice that qq2 doesn't use async/await (is this what you meant?). Thus, it seems that to support asyncio ApiClient, we only need to implement the ApiClient and there is no need to touch anything else. Even more, this can be implemented as a standalone Python package, so Codegen can only get updated README to show how to enable asyncio ApiClient (pip install swagger-codegen-asyncio + UsersApi(api_client=swagger_codegen_asyncio.ApiClient()))

P.S. The suggestion about a separate package for AsyncIO is a bit awkward and complicates maintenance, but it still seems to be a valid approach.

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

No branches or pull requests

4 participants