Allow injecting reqwest client

[ernesto-jimenez]

Hey @LevitatingBusinessMan, thanks for the crate!

Would you be up for allowing injecting a reqwest client into the client?

It would allow setting up consumers to set-up middleware.

I was thinking maybe: openai_rust::Client::new_with_client(key, reqwest_client)

[LevitatingBusinessMan]

Hey @ernesto-jimenez,

I like this idea for adding custom proxies and User-Agents and such. But as reqwest clients cannot be modified, my API would have to store the api key in the Client struct and add it to each request.

I did this here: 060ccce

I am unsure if this is the smartest way to implement this. What would you think about passing a ClientBuilder instead? The crate would just add the bearer token.

I am open to ideas.

[ernesto-jimenez]

Thanks for the speedy reply!

I only suggested injecting a client because, AFAIK, reqwest clients handle connection pooling with keepalive. Re-using the reqwest client across openai clients could bring some performance benefits.

With that being said, I just started learning Rust a few days ago, so it's possible I might be missing something.

[LevitatingBusinessMan]

I only suggested injecting a client because, AFAIK, reqwest clients handle connection pooling with keepalive. Re-using the reqwest client across openai clients could bring some performance benefits.

That's true but I am not sure if many applications would use multiple openai clients as they can be reused. The only reason to do so would be if you wanted to use multiple api keys.

[ernesto-jimenez]

Probably not many, but there will certainly be some.

That's what I'm planning to do myself.

[LevitatingBusinessMan]

For now you can configure your Cargo.toml to use the external_client branch of this crate.

openai-rust = { git = "https://github.com/LevitatingBusinessMan/openai-rust.git", branch = "external_client" }

And I'll try to make a decision about how to do this.

[ernesto-jimenez]

@LevitatingBusinessMan FYI, took a stab at this in PR #3

[LevitatingBusinessMan]

Partially fixed with in 0.5.0, however this does not support clients from the middleware crates and the likes yet.

[LevitatingBusinessMan]

I added a feature in a feature branch that allows you to use a client with middleware.

aef2529

[LevitatingBusinessMan]

As for rvcr, it currently does not work with the new reqwest_middleware version... ChorusOne/rvcr#5.

And this kind of stuff is exactly why I am so hesitant with involving reqwest_middleware with the project.

[ernesto-jimenez]

I wouldn't couple this crate with reqwest_middleware if it were me.

Accepting a trait means that people can use reqwest, reqwest_middleware, their own adapter for some other client library, or a mock.

An alternative approach would be, rather than embedding reqwest::Client directly into the client, to create a small trait to make HTTP requests. Then have this crate contain a default implementation with reqwest (maybe behind a flag?), but allow others to implement and inject their own.

Honestly, coming from Go all this seems quite cumbersome. I'm used to packages aligning with the net/http interfaces from the standard library, allowing for lots of interoperability and extensibility between packages.

[LevitatingBusinessMan]

Accepting a trait means that people can use reqwest, reqwest_middleware, their own adapter for some other client library, or a mock.

Yes but with traits we get into the issue again of the client having a generic type.