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

Initialize Client with custom HTTP client, config, and backoff #193

Closed
GabrielBianconi opened this issue Mar 1, 2024 · 3 comments · Fixed by #197
Closed

Initialize Client with custom HTTP client, config, and backoff #193

GabrielBianconi opened this issue Mar 1, 2024 · 3 comments · Fixed by #197
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@GabrielBianconi
Copy link
Contributor

GabrielBianconi commented Mar 1, 2024

Initializing a Client with a custom HTTP client always requires first creating a useless reqwest::Client in Client::with_config.

This adds a bit of useless overhead due to reqwest's TLS/DNS functionality, which can be non-trivial for certain low-latency applications.

Would you be open to adding an additional method along the lines of?

impl<C: Config> Client<C> {
    /// Create client with your own HTTP client, config, and backoff 
    pub fn build(http_client: reqwest::Client, config: C, backoff: backoff::ExponentialBackoff) -> Self {
        Self {
            http_client,
            config,
            backoff,
        }
    }

...
@64bit
Copy link
Owner

64bit commented Mar 1, 2024

Thank you for detailed description. Definitely open to it, PR is welcome!

I'm curious what happens with TLS/DNS on creating a new reqwest client?

@GabrielBianconi
Copy link
Contributor Author

Great, just sent a PR #197.

During initialization, among other things, a reqwest client seems to initialize its TLS backend ref. In certain scenarios (at least for me: tiny VM with shared core + legacy backend), this seems to take a non-trivial amount of time. I observed ~30ms earlier this week. And with the current async-openai implementation, it wasn't possible to easily circumvent this dummy initialization.

Thanks!

@64bit
Copy link
Owner

64bit commented Mar 2, 2024

I see, thank you for sharing!

@64bit 64bit closed this as completed in #197 Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants