-
Notifications
You must be signed in to change notification settings - Fork 0
Jan Server can do Models routing #197
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
Conversation
defer writer.Close() | ||
|
||
reqBody := request | ||
reqBody.Stream = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not overwrite the function arguments.
} | ||
|
||
func (c *Client) CreateChatCompletionStream(ctx context.Context, apiKey string, request openai.ChatCompletionRequest) (io.ReadCloser, error) { | ||
reader, writer := io.Pipe() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return an error early.
re-write to something like:
req := RestyClient.R().SetBody(request)
resp, err := req.
SetContext(ctx).
SetHeader("Authorization", fmt.Sprintf("Bearer %s", apiKey)).
SetHeader("Accept-Encoding", "identity"). // <- no gzip
SetHeader("Content-Type", "application/json").
SetDoNotParseResponse(true).
Post(c.baseURL + "/chat/completions")
if err != nil {
return nil, err
}
if resp.IsError() {
return nil, err
}
reader, writer := io.Pipe()
go func() {
defer writer.Close()
defer resp.RawResponse.Body.Close()
// Pipe the SSE bytes through; caller should parse "data: ..." lines.
if _, err = io.Copy(writer, resp.RawResponse.Body); err != nil {
writer.CloseWithError(err)
return
}
}()
return reader, nil
return &response, nil | ||
} | ||
|
||
func (c *Client) CreateChatCompletionStream(ctx context.Context, apiKey string, request openai.ChatCompletionRequest) (io.ReadCloser, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't OpenRouter, Jan-Inference, and Gemini have the same handler for streaming responses??
@@ -0,0 +1,62 @@ | |||
package admin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unnecessary endpoints and services.
- the operation belongs to DevOps/SRE
func (p *ModelProvider) EtoD() *domain.ModelProvider { | ||
if p == nil { | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unreachable code.
If it's AI-generated, please review the result yourself next time.
func (r *ModelProviderGormRepository) Update(ctx context.Context, provider *domain.ModelProvider) error { | ||
model := dbschema.NewSchemaModelProvider(provider) | ||
tx := r.db.GetTx(ctx) | ||
return tx.WithContext(ctx).Model(&dbschema.ModelProvider{}). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
query.ModelProvider.WithContext(ctx).Save(model)
if err != nil { | ||
reqCtx.AbortWithStatusJSON(http.StatusInternalServerError, responses.ErrorResponse{ | ||
Code: "5c6c9f1f-0d6a-40a4-9fa6-823f19f8391d", | ||
Error: "failed to load provider", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ErrorInstance: err
encryptionSecret string | ||
} | ||
|
||
type CreateOrganizationProviderInput struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you can just use ModelProvider(domain)
Active bool | ||
} | ||
|
||
type UpdateOrganizationProviderInput struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you can just use ModelProvider(domain)
return | ||
} | ||
|
||
input := modelprovider.UpdateOrganizationProviderInput{PublicID: provider.PublicID} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about adding a function to generate ModelProvider from the request?
It's a common pattern to convert objects to entities in DDD.
func(req)GetModelProvider() ModelProvider {
...
}
req.GetModelProvider()
return | ||
} | ||
|
||
provider, err := route.fetchOrganizationProvider(reqCtx.Request.Context(), orgEntity.ID, reqCtx.Param("provider_id")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reqCtx.Param("provider_id") check bad request
authService *auth.AuthService | ||
providerService *modelprovider.ModelProviderService | ||
projectService *project.ProjectService | ||
cache *cache.RedisCacheService |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The route is supposed to integrate with domain services; we should move the cache to the service level.
Although named RedisCacheService, it is actually a repository.
route.invalidateOrganizationModelsCache(reqCtx.Request.Context(), orgEntity.ID) | ||
unlinkProviderModelsCache(reqCtx.Request.Context(), route.cache, updated.PublicID, "organization provider") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move them to providerService
, maybe just after UpdateOrganizationProvider
route.invalidateOrganizationModelsCache(reqCtx.Request.Context(), orgEntity.ID) | ||
if provider.ProjectID != nil { | ||
cacheKey := fmt.Sprintf(cache.ProjectModelsCacheKeyPattern, *provider.ProjectID) | ||
if err := route.cache.Unlink(reqCtx.Request.Context(), cacheKey); err != nil { | ||
logger.GetLogger().Warnf("organization provider: failed to invalidate project cache for project %d: %v", *provider.ProjectID, err) | ||
} | ||
} | ||
unlinkProviderModelsCache(reqCtx.Request.Context(), route.cache, provider.PublicID, "organization provider") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move them to providerService services
// @Failure 401 {object} responses.ErrorResponse "Unauthorized" | ||
// @Failure 404 {object} responses.ErrorResponse "Provider not found" | ||
// @Failure 500 {object} responses.ErrorResponse "Internal server error" | ||
// @Router /v1/organization/projects/{proj_public_id}/providers/{provider_id} [get] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
projects
return | ||
} | ||
|
||
provider, err := route.fetchProjectProvider(reqCtx.Request.Context(), orgEntity.ID, projectEntity.ID, reqCtx.Param("provider_id")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reqCtx.Param("provider_id")
route.invalidateProjectModelsCache(reqCtx.Request.Context(), projectEntity.ID) | ||
route.invalidateOrganizationModelsCache(reqCtx.Request.Context(), orgEntity.ID) | ||
unlinkProviderModelsCache(reqCtx.Request.Context(), route.cache, provider.PublicID, "project provider") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The route should not be concerned with the cache.
if provider == nil || provider.OrganizationID == nil || *provider.OrganizationID != organizationID { | ||
return nil, nil | ||
} | ||
if provider.ProjectID == nil || *provider.ProjectID != projectID { | ||
return nil, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return error base on the context?
func (route *ProjectProviderRoute) invalidateOrganizationModelsCache(ctx context.Context, organizationID uint) { | ||
if route.cache == nil { | ||
return | ||
} | ||
cacheKey := fmt.Sprintf(cache.OrganizationModelsCacheKeyPattern, organizationID) | ||
if err := route.cache.Unlink(ctx, cacheKey); err != nil { | ||
logger.GetLogger().Warnf("project provider: failed to invalidate organization cache for org %d: %v", organizationID, err) | ||
} | ||
} | ||
|
||
func unlinkProviderModelsCache(ctx context.Context, cacheService *cache.RedisCacheService, providerID string, logPrefix string) { | ||
if cacheService == nil { | ||
return | ||
} | ||
trimmed := strings.TrimSpace(providerID) | ||
if trimmed == "" { | ||
return | ||
} | ||
cacheKey := fmt.Sprintf("%s:%s", cache.ModelsCacheKey, trimmed) | ||
if err := cacheService.Unlink(ctx, cacheKey); err != nil { | ||
logger.GetLogger().Warnf("%s: failed to invalidate provider models cache for provider %s: %v", logPrefix, trimmed, err) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove cache from route
Added Cloud Provider with proxy routing
Added/Updated API
Providers (org / project)
Model catalog & discovery
Inference (proxy/routing)
Caching (admin)
Bug fixes