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

Allow for custom Ktor HttpClientPlugin installations #238

Closed
rasharab opened this issue Sep 27, 2023 · 5 comments
Closed

Allow for custom Ktor HttpClientPlugin installations #238

rasharab opened this issue Sep 27, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@rasharab
Copy link
Contributor

Feature Description

Allow for OpenAI HttpClient to install custom Ktor HttptClientPlugins.

Problem it Solves

Our team has custom plugins that upload telemetry for various ktor clients.
They extend HttpClientPlugin.

Currently, we're using reflection to install our own HttpClient/HttpTransport to openai-kotlin's OpenAIApi instance.
This is untenable and we'd like for first-class support.

Proposed Solution

There are several ways of doing this:

Solution1:
Add configuration parameter in OpenAIConfig called plugins:

public val plugins: List<HttpClientPlugin<*, *>

And install those plugins in HttpClient.

        config.plugins.forEach {
            install(it)
        }

Solution2:

Add a configuration parameter for a lambda to do custom configuration of the HttpClientConfig. More error prone.

public val configure: HttpClientConfig<*>.() -> Unit = {}

And run that configuration in HttpClient:

config.configure.invoke(this)

I will probably get a PR up to address this soon, but don't want to do unnecessary work if the maintainer doesn't want this.

@rasharab rasharab added the enhancement New feature or request label Sep 27, 2023
@rasharab rasharab changed the title Allow for custom httpClient plugin installations Allow for custom Ktor HttpClientPlugin installations Sep 27, 2023
@aallam
Copy link
Owner

aallam commented Sep 27, 2023

I've been trying to keep Ktor as an implementation detail, but I've been seeing more requests to configure the client. So, why not? Solution 2 looks good to me!

@rasharab
Copy link
Contributor Author

rasharab commented Sep 27, 2023

Completely understand your thought process there.
Alas, can't think of a super clean way of doing this otherwise.
I'll forward a pr shortly.
Thanks for creating this package...

@rasharab
Copy link
Contributor Author

First stab:
https://github.com/aallam/openai-kotlin/pull/239/files

@aallam
Copy link
Owner

aallam commented Sep 28, 2023

Version 3.4.2 is released with your changes! thanks again for your contribution! 🎉

@aallam aallam closed this as completed Sep 28, 2023
@rasharab
Copy link
Contributor Author

Thank you again!

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

No branches or pull requests

2 participants