-
Notifications
You must be signed in to change notification settings - Fork 195
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
Enable polymorphism / dynamic dispatch for AzureConfig and OpenAIConfig #106
Comments
Also, I can't push branches: (base) ➜ async-openai git:(deip/common-config-trait) git push --set-upstream origin deip/common-config-trait
ERROR: Permission to 64bit/async-openai.git denied to deipkumar.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists. Do I need to be added as a contributor? |
Please use fork for personal branches and contributions |
@64bit Why did you want pub struct Client {
...
config: Box<dyn Config>,
...
} No need to propagate the generics everywhere |
I do not recall :) Your idea of Box-ing it is actually good. Not sure what the rippling effects would be but worth exploring. |
Having a trait with associated type would allow the idea to work using only generics (no need for dynamic dispatch): // Only Config seems to be polymorphic here.
// In any case, one can always have client: impl GenericClient<Config=C> below (see my comment on #125 below)
fn use_client<C: GenericConfig>(client: Client<C>) {
let models = client.models();
// ...
}
fn main() {
let openai_config = OpenAIConfig::default();
let openai_client = Client::with_config(openai_config);
// No need for dynamic dispatch: the types in your problem are known at compile time. No boxing means zero cost dispatch at run time.
use_client(openai_client);
let azure_config = AzureConfig::default();
let azure_client = Client::with_config(azure_config);
use_client(azure_client);
} Related to a comment of mine (#125 (comment)). Keep in mind that the suggestion in the comment does allow dynamic dispatch using a simple design widely used in the Rust world ( |
Would like to enable something like this:
The text was updated successfully, but these errors were encountered: