Add async support

[Andrew-Chen-Wang]

Fixes #98

Adds asyncio support for API. Does not support CLI (does not make sense).

I haven't tested file upload. aiohttp does not include the files parameter like requests, so I'm using requests' private utility function in the ApiRequestor

Usage (notice the a prefix, used in CPython lib, Django, and other libs):

import openai

openai.api_key = "sk-..."

async def main():
    await openai.Completion.acreate(prompt="This is a test", engine="text-ada-001")

Installation of aiohttp can include speedups that aren't included in setup.py. aiohttp is a required package, but it doesn't have to be. We'd just need to remove the typing.

[MaxTretikov]

Love this addition, hope it gets merged. Wanted to add that I seem to have been getting Unclosed client session errors when using this:

client_session: <aiohttp.client.ClientSession object at 0x104306b00>
Unclosed client session

[Andrew-Chen-Wang]

Thanks for catching that @MaxTretikov ! This should be resolved in the latest commit

[Andrew-Chen-Wang]

Thanks for your time and the review @ddeville !

[ddeville]

Would love for @hallacy to take a look before we merge though.

[Andrew-Chen-Wang]

thanks @ddeville and @hallacy !!

in the future to possibly prevent further duplication of code, I highly recommend two resources:

• SansIO approach to implementing a sync/async lib (ex impl)
• A simpler method can be found in redis-py itself. In this method, we have the commands folder (similar to the api_resources folder) and the redis.Redis and redis.asyncio.Redis class (similar to ApiRequestor). The Redis classes implement their own execute_command method and a command from api_resources runs execute_command. The reason for this PR's amount of duplication of code is because of útil.convert_to_openai_object. If that function was instead moved to ApiRequestor.request, you're left with a mixin that is potentially compatible for both sync and async classes. But it can be tricky depending on future implementation and CLI compatibility

[ddeville]

Thanks for the links @Andrew-Chen-Wang this is super interesting. From a quick glance, it looks like it works more or less like this:

class BaseExecutor:
    def execute_cmd(...):
        ...

class SyncExecutor(BaseExecutor):
    def execute_cmd(...):
        return get(...)

class AsyncExecutor(BaseExecutor):
    async def execute_cmd(...):  # not sure how typing works here though since they return different types
        return await get(...)

class Model:
    def __init__(self, client: BaseExecutor):
        self.execute_cmd = client.execute_cmd
   
   def something(self, ...):
       request = ...
       return self.execute_cmd(request)

And then you would use it like that

# from a sync context
def main():
    model = Model(SyncExecutor())
    ret = model.something()

# from an async context
async def main():
    model = Model(AsyncExecutor())
    ret = await model.something()

(so basically BaseExecutor is a mixin and here convert_to_openai_object is a problem because it is called in the calling code rather than in ApiRequestor which means that Model would need to be able to await the result if the executor was async rather than just returning the coroutine?)

Am I reading this right?

The only issue I can think of is that the methods on Model are not marked as async but still return coroutines in the async case which is not super clear and might lead to problems where callers forget to await on their result. Has this been a problem in your experience?

[Andrew-Chen-Wang]

You got it right!

To answer the question on typing, in redis-py, we make a type Union with Awaitable and result type.

However the reason I make these suggestions is because the example usage you've got it quite unwieldy, hence a preference for the current implementation in this PR. A better implementation would be to have something similar to the Redis class, like ApiRequestor but named OpenAI, that could call commands like OpenAI().Completion(...)

But that's essentially a complete rewrite that I'm. not quite for at the moment. Would prefer to have the PR merged as it seems like a complete rewrite would be necessary (looking at the stripe sdk rn which this lib is forked from; for reference: https://github.com/stripe/stripe-python/issues/715#issuecomment-794881701)

[ksromero]

When is the release of this?