[BUG][Python] async_req=True doesn't work with asyncio

[TBBle]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• What's the version of OpenAPI Generator used?
• Have you search for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Bounty to sponsor the fix

Description

When calling a generated API, the keyword parameter async_req=True is supposed to cause the request to be performed asynchronously, i.e. using multiprocessing.apply_async to handle __call_api instead of directly calling it.

However, in the --library asyncio target, __call_api is a coroutine, which multiprocessing.apply_async does not know how to handle.

openapi-generator version

4.3.0-SNAPSHOT, pulled as follows:

PS C:\Users\paulh> docker pull openapitools/openapi-generator-cli
Using default tag: latest
latest: Pulling from openapitools/openapi-generator-cli
709515475419: Already exists
38a1c0aaa6fd: Already exists
cd134db5e982: Already exists
044751432162: Pull complete
d41fd233bfb5: Pull complete
b9e8842b3734: Pull complete
Digest: sha256:76b6fc8b94789382e1d91319b787545ec5b7f29c3709c2c14bc47eab68d3a871
Status: Downloaded newer image for openapitools/openapi-generator-cli:latest
docker.io/openapitools/openapi-generator-cli:latest

PS C:\Users\paulh> docker image ls openapitools/openapi-generator-cli
REPOSITORY                           TAG                 IMAGE ID            CREATED             SIZE
openapitools/openapi-generator-cli   latest              e74d7eb8269d        29 minutes ago      135MB

I don't believe this is a recent regression, the code hasn't been touched since it was introduced in #107 in 2018.

OpenAPI declaration file content or url

https://raw.githubusercontent.com/openapitools/openapi-generator/master/modules/openapi-generator/src/test/resources/2_0/petstore.yaml

Command line used for generation

docker run --rm -v ${PWD}:/local openapitools/openapi-generator-cli generate -i https://raw.githubusercontent.com/openapitools/openapi-generator/master/modules/openapi-generator/src/test/resources/2_0/petstore.yaml -g python --library asyncio -o /local/out/python

Steps to reproduce

After code-gen, create and run the following Python script (which includes workaround for #5538)

import asyncio
import sys

sys.path.append("out/python")
import openapi_client
from openapi_client.rest import ApiException

async def main():
    configuration = openapi_client.Configuration()

    with openapi_client.ApiClient(configuration) as api_client:

        api_instance = openapi_client.PetApi(api_client)

        try:
            # Trigger a 404 search result
            result = api_instance.get_pet_by_id("nosuchpet", async_req=True)
            result.wait()
        except ApiException as e:
            print(f"get_pet_by_id failed: {e.reason}")
            assert e.status == 404
            raise e
        finally:
            await api_client.rest_client.pool_manager.close()

def monkey():
    from openapi_client.rest import RESTClientObject

    del RESTClientObject.__del__


monkey()

if __name__ == "__main__":
    try:
        asyncio.run(main())
    except ApiException as e:
        assert e.status == 404
        pass

Actual output

C:\Program Files\Python37\lib\asyncio\events.py:88: RuntimeWarning: coroutine 'ApiClient.__call_api' was never awaited
  self._context.run(self._callback, *self._args)
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

Notably, the request was never sent.

Expected output

get_pet_by_id failed: Not Found

Related issues/PRs

None I'm aware of.

Suggest a fix

Simple fix is to reject the async_req keyword in asyncio builds, since we're already "async", even though it's not threaded.

A more complex fix would be to have the asyncio target use concurrent.futures.ThreadPoolExecutor as the Pool, and asyncio.loop.run_in_executor instead of multiprocessing.apply_async to run the request in the pool.

That will then return an awaitable, which means the API for the caller is the same whether async_req is True or False, avoiding the ApplyResult needed in the other library's use of async_req.

---

Note that

result = api_instance.get_pet_by_id("nosuchpet", async_req=True)
await result.get()

works correctly, but it has actually performed the HTTP request on the main thread, and so all it did in the thread pool was create a coroutine object. So it's equivalent to async_req=False, but more wasteful.

---

So the open question is, does async_req indicate "threaded" or "async"?

[auto-labeler]

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

[paololucchino]

Hello, I'd like second this issue.

Because of the bug in how the async_req flag works, the client currently can really only work asynchronously. Having it work both as an async and sync would be a very handy functionality.

Is someone working on this issue?

Thanks!

[wing328]

@paololucchino we welcome contributions to fix it.

Also worth mentioning the python generator in 4.x has been renamed to python-legacy generator in 5.x while the python generator in 5.x has been refactored.

[lihaif]

It will be nice if the generated code can also pass a callback/error_callback(https://docs.python.org/3/library/multiprocessing.html#multiprocessing.pool.Pool.apply_async) when async_req is true, which makes it easier to write logic to handle the reply.