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

[WIP] fix: make OpenAiProxyFactory closable #26

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

Conversation

mscheong01
Copy link
Owner

resolves #25

Copy link
Collaborator

@davin111 davin111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no need to implement close() for OpenAiApiClient, because spring WebClient does it in its own way?

I read this sentence in #25.

This did not happen when I used OpenAiApiClient defined in the spring boot starter that uses a spring webclient.

response.body().close()
}
} catch (e: java.lang.Exception) {
// logger.warn("Failed to quietly close Response", e)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line needed?

@mscheong01
Copy link
Owner Author

mscheong01 commented Jun 13, 2023

@davin111 After doing some research, here is what I learned:

  • Spring WebClient, which is based on Reactor Netty uses daemon threads for handling network connections -> this is why tests exit normally
  • OkhttpClient uses non-daemon threads for network connection by default, making the tests wait for a while to exit

so instead of using this implementation, we could customize the OkhttpClient to use daemon threads for executors (After doing some research on whether there are other side effects)
ref: square/okhttp#4029, chatgpt

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.

Provide a way to close OkHttpClient when using DefaultOkHttpOpenAiClient
2 participants