dylanshine / openai-kit

A community Swift package used to interact with the OpenAI API
https://platform.openai.com/docs/api-reference
MIT License
712 stars 113 forks source link

Update README.md with More Thorough Documentation #46

Open julianworden opened 1 year ago

julianworden commented 1 year ago

I started working with this library because it was listed here, but the documentation was quite minimal and I ended up having to experiment a lot in order to figure out how to do most things. It's possible that not all of the info in this PR is 100% correct, but code examples such as these would've been very helpful for me when I first started using this library so I'm confident that at least some of this will be useful.

One thing I did remove was this section from the setup instructions:

defer {
    // it's important to shutdown the httpClient after all requests are done, even if one failed. See: https://github.com/swift-server/async-http-client
    try? httpClient.syncShutdown()
}

While I'm sure it'd be important to include this block, I cannot figure out how to actually use it in the context of an app. If I call it when the app is first initialized, I get an error message after sending my first message. Currently, I have a sample app running just fine without this block, so if someone would like to incorporate this block into my examples I'll leave that up to you.

Thanks!

dylanshine commented 1 year ago

So in your example, you would ideally want to clean up the httpClient you initialize in your init(), once your OpenAiService object is deinited. A strategy that you could use is to initialize a new HTTPClient on the app startup, and continually pass it into instances of your OpenAiService. Once your application is be be terminated, you could then clean up your HTTPClient with a shutdown.

julianworden commented 1 year ago

So in your example, you would ideally want to clean up the httpClient you initialize in your init(), once your OpenAiService object is deinited. A strategy that you could use is to initialize a new HTTPClient on the app startup, and continually it into instances of your OpenAiService. Once your application is be be terminated, you could then clean up your HTTPClient with a shutdown.

Ah, that makes a lot more sense, I didn't think to put the syncShutdown call in the deinit. My thought process was that, since the OpenAiService will be alive only throughout the lifetime of the app, once the app is quit the HTTPClient won't be doing anything anyway so there's no need to do anything to it in the deinit. I'm guessing there is something going on behind the scenes with HTTPClient even when the app isn't active then? Could you shed some more light on that so I could revise the documentation accordingly?

joshgalvan commented 1 year ago

Once your application is be be terminated, you could then clean up your HTTPClient with a shutdown.

What do you suggest is the best way to do this? I'm reading a lot of information about executing code on app termination and that it is not reliable. Should one use scenePhase? Or an older UIApplicationDelegate method? Do you just suggest creating a deinit on some HTTPClient singleton? Dcoumentation that would reflect the best practices for this would be helpful.

I'm currently using an HTTPClient singleton with a deinit that calls syncShutdown() as trying to do this via phases wasn't very successful.

final class HTTPClientService {

    let client: HTTPClient

    static let shared = HTTPClientService()

    private init() {
        let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)
        self.client = HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup))
    }

    deinit {
        try? client.syncShutdown()
    }
}

Then every time I need to use OpenAIKit.Client I pass HTTPClientService.shared.client into the init.

julianworden commented 1 year ago

Once your application is be be terminated, you could then clean up your HTTPClient with a shutdown.

What do you suggest is the best way to do this? I'm reading a lot of information about executing code on app termination and that it is not reliable

To add onto @joshgalvan's comment, like I mentioned above, I'm also interested in why calling syncShutdown() when the app is terminated is necessary in the first place. I would expect that the networking code wouldn't be doing anything while the app is in the background or is inactive anyway, and that once it's quit and restarted the HTTPClientService would get initialized normally and the app would run. So what's the point?

@joshgalvan To answer part of your your question, yes, I've found that UIApplication.willResignActiveNotification and similar are much more reliable than monitoring scenePhase changes. However, as far as I know, there is no way to monitor whether the app was terminated with one of these notifications. I believe you can only get notified above the app moving into or out of the foreground, background, or inactive states, hence part of my (and your) confusion.