Skip to content

[WIP] fix: make OpenAiProxyFactory closable#26

Open
mscheong01 wants to merge 1 commit into
mainfrom
make-OpenAiProxyFactory-closable
Open

[WIP] fix: make OpenAiProxyFactory closable#26
mscheong01 wants to merge 1 commit into
mainfrom
make-OpenAiProxyFactory-closable

Conversation

@mscheong01

Copy link
Copy Markdown
Owner

resolves #25

@davin111 davin111 left a comment

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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

mscheong01 commented Jun 13, 2023

Copy link
Copy Markdown
Owner Author

@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