openai-python icon indicating copy to clipboard operation
openai-python copied to clipboard

Update README for fastapi integration

Open brian-goo opened this issue 1 year ago • 5 comments

  • [x] I understand that this repository is auto-generated and my pull request may not be merged

Changes being requested

Additional context & links

I guess how to integrate with frameworks like FastAPI should be documented to avoid confusion. https://github.com/openai/openai-python/issues/820

brian-goo avatar Dec 01 '23 03:12 brian-goo

Maybe that's overly complex, though?

It also works to just to instantiate a client in some module, and use that in places where you need it, without the singleton class nor depends or anything such. If you don't need a connection pool or something.

antont avatar Dec 01 '23 05:12 antont

Well, I kind of intentionally put this right below Module-level client

Ah sorry, I missed that it was there, was just reading the diff.

I think this way it makes a lot of sense, to have the trivially simple option, and then this fully featured way.

antont avatar Dec 01 '23 07:12 antont

We need a full-featured async client for our production server and I believe somebody else needs it as well.

Actually, if you don't mind me asking, what's the need exactly? Your code seems to do a client per session, no? Why?

I'm also using FastAPI and so far with just the client-in-module approach, and am not (yet) aware of problems it would introduce. A single client gets reused within a process, and is closed when the process shuts down. AFAIK reusing the same client for multiple sessions is fine.

I'm not saying that there would not be problems, would just like to learn what exactly, and I guess that would be useful for the documentation too.

antont avatar Dec 01 '23 08:12 antont

Like @antont mentioned you should be able to share the same client between requests, is there a reason you set one up per-session?

This then simplifies the code a lot, so it should look something like this:

from contextlib import asynccontextmanager
from fastapi import FastAPI
from openai import AsyncOpenAI

client = AsyncOpenAI()

@asynccontextmanager
async def lifespan(app: FastAPI):
    yield
    await client.close()

app = FastAPI(lifespan=lifespan)

RobertCraigie avatar Dec 01 '23 14:12 RobertCraigie

@brian-goo The reason why I included the wrapper class is that a wrapper makes it easier to handle states of multiple initialized clients

for multiple api keys etc AND fits into the dependency injection system of FastAPI.

Right, figures.

I'd think that a more complex example is useful, for example one that indeed gives different clients for different request handlers.

The minimal case seems weird as it does not really have any benefit over just using a single client without the Dependency.

The client supports using different API keys for different requests, though, with a single client, so I'm not sure what would be a good simple example case for a wrapper.

Just my 2 cents, lib authors may see this differently.

antont avatar Dec 02 '23 20:12 antont