[OPIK-610] multi models support infrastructure #957
Conversation
There was a problem hiding this comment.
This looks in the good direction in my opinion.
I encourage to keep abstracting the minimum parts that differ per LLM provider, mostly the client instantiation, the calls (normal and streaming) and converting requests and responses from/to the OpenAI format.
Everything out of this, leave it in common classes/services, no need for interfacing them if not needed. This is to avoid unnecessary complexity.
apps/opik-backend/src/main/java/com/comet/opik/domain/llmproviders/LlmProviderFactory.java
Outdated
Show resolved
Hide resolved
.../opik-backend/src/main/java/com/comet/opik/domain/llmproviders/LlmProviderStreamHandler.java
Outdated
Show resolved
Hide resolved
|
+1 to Andres's comment, there's still a lot that can be moved to a common ancestor class. |
|
Do you think it's still relevant to the latest version? |
idoberko2
force-pushed
the
ido/opik-610-support-anthropic-models
branch
from
8b7025b to
3bfb047
Compare
apps/opik-backend/src/main/java/com/comet/opik/domain/llmproviders/LlmProviderFactory.java
Outdated
Show resolved
Hide resolved
apps/opik-backend/src/main/java/com/comet/opik/domain/llmproviders/OpenAi.java
Outdated
Show resolved
Hide resolved
apps/opik-backend/src/main/java/com/comet/opik/domain/llmproviders/OpenAi.java
Show resolved
Hide resolved
| throw llmProviderClient.mapRuntimeException(runtimeException); | ||
| if (llmProviderClient.getHttpExceptionClass().isInstance(runtimeException.getCause())) { | ||
| int statusCode = llmProviderClient.getHttpErrorStatusCode(runtimeException); | ||
| if (statusCode >= 400 && statusCode <= 499) { |
There was a problem hiding this comment.
you can use the following code to check for response families instead of doing this yourself:
Response.Status.Family.familyOf(statusCode) == Response.Status.Family.CLIENT_ERROR)
works well for success response as well (I think I saw the same type of comparison for 200 somewhere in your code)
Details
This lays the ground for adding additional LLM provider integrations.
Issues
OPIK-610