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

Update README.md with More Thorough Documentation #46

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

julianworden
Copy link

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!

Adds more thorough documentation to OpenAiKit's README.md file.
@dylanshine
Copy link
Owner

dylanshine commented May 12, 2023

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
Copy link
Author

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
Copy link
Contributor

joshgalvan commented May 30, 2023

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
Copy link
Author

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.

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

Successfully merging this pull request may close these issues.

None yet

3 participants