Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Add GenericProviderConfig.ServiceName#63626

Merged
chrsmith merged 1 commit into
mainfrom
chrsmith/fix-design-flaw_ii
Jul 3, 2024
Merged

Add GenericProviderConfig.ServiceName#63626
chrsmith merged 1 commit into
mainfrom
chrsmith/fix-design-flaw_ii

Conversation

@chrsmith

@chrsmith chrsmith commented Jul 3, 2024

Copy link
Copy Markdown
Contributor

While trying to use the types.ModelConfiguration data to configure completion providers, it became clear I missed something.

We expose the types.GenericProviderConfig as a bare-bones way to express LLM provider configuration, and it does great. But just knowing the (endpoint, access token) isn't enough to actually serve requests.

We need to know the shape of API requests. e.g. if we should pass that endpoint/access to an OpenAI-compatible, Fireworks-compatible, or Google-compatible API provider.

NOTE: An alternative approach would be to not add this field to GenericProviderConfig entirely, and instead add AnthropicProvider *GenericProviderConfig, GoogleProvider *GenericProviderConfig, etc. to the ServerSideProviderConfig. And essentially have the "service name" be implicit based on which field of ServerSideProviderConfig you found the object.

There are advantages to that approach, but this seemed a bit cleaner IMHO. WDYT?

Test plan

Added new unit tests

Changelog

NA

@chrsmith chrsmith requested a review from emidoots July 3, 2024 21:06
@cla-bot cla-bot Bot added the cla-signed label Jul 3, 2024
@chrsmith chrsmith merged commit 08252b1 into main Jul 3, 2024
@chrsmith chrsmith deleted the chrsmith/fix-design-flaw_ii branch July 3, 2024 21:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants