feat(router): add OpenAI-compatible /v1/files API - #1465
feat(router): add OpenAI-compatible /v1/files API#1465katara-Jayprakash wants to merge 4 commits into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Enable batch file upload/list/get/delete in kthena-router. Signed-off-by: katara-Jayprakash <katarajayprakash@icloud.com>
8e60356 to
a83170a
Compare
|
/cc @FAUST-BENCHOU @LiZhenCheng9527 @YaoZengzeng @hzxuzhonghu ptal as this is foundational pr for adding the feat/openai-compatible files apis. |
Signed-off-by: katara-Jayprakash <katarajayprakash@icloud.com>
9374de7 to
c578ff1
Compare
| connectorFactory *connectors.Factory | ||
|
|
||
| // OpenAI-compatible Files API (batch control plane). Nil store disables it. | ||
| filesHandler *batch.Handler |
There was a problem hiding this comment.
Why should file storage and asynchronous tasks belong to the Router?
There was a problem hiding this comment.
This PR only adds the /v1/files endpoint. The worker is not included yet.I put it in the router because clients already use the router for the OpenAI APIs like /v1/chat/completions and /v1/models. Since the Files is part of the same API, it felt better to keep it there so auth and logging work the same way.
The main reason is in the next PR. The batch worker will send each JSONL line through the existing routing (doLoadbalance / scheduler) and If the Files and Batches were in a separate service, we'd have to build the same routing logic again, and it wouldn't share the same backend load information. and the storage is behind a FileStore interface, so later we can change from local disk to shared storage without moving the API out of the router.
| batchTTL time.Duration | ||
|
|
||
| mu sync.RWMutex | ||
| files map[string]*storedFile |
There was a problem hiding this comment.
The current implementation writes file contents to the local disk of each Router Pod, while metadata is stored in the process's own in-memory map. This approach may cause issues with multi-replica routers.
There was a problem hiding this comment.
and all records will lost after restart
| } | ||
|
|
||
| now := time.Now().Unix() | ||
| expiresAt := now + int64(s.batchTTL.Seconds()) |
There was a problem hiding this comment.
same fixed it in next commit
|
|
||
| // Environment variable keys for batch file storage configuration. | ||
| const ( | ||
| EnvFilesDir = "KTHENA_BATCH_FILES_DIR" |
There was a problem hiding this comment.
may also need to change the helm
There was a problem hiding this comment.
Agreed done in the latest commit.
There was a problem hiding this comment.
I'm not familiar with these compatible api,do we also need e2e test?
There was a problem hiding this comment.
no i don't think so, as i already added unittest this would going to be more then enough
Honor expires_at on get/list/open and expose opt-in router env/volume settings so operators can enable /v1/files via the chart. Signed-off-by: katara-Jayprakash <katarajayprakash@icloud.com>
Signed-off-by: katara-Jayprakash <katarajayprakash@icloud.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
LiZhenCheng9527
left a comment
LiZhenCheng9527
left a comment
There was a problem hiding this comment.
This file is stored in the memory of the router. If there are multiple router replicas, whether the requests before and after the request are forwarded by the same router pod. Otherwise, the file fails to be obtained.
4 participants
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adds OpenAI-compatible
/v1/filessupport inkthena-routeras the foundation for Batch API workloads.purpose=batchFileStoreinterface/v1/models), before model request parsingKTHENA_BATCH_FILES_DIRis set; optionalKTHENA_BATCH_MAX_FILE_BYTES(default 200MiB) andKTHENA_BATCH_FILE_TTL(default 30d)This unlocks offline batch input/output file management inside the router (no separate service) and prepares for the follow-up
/v1/batches+ worker PR.Which issue(s) this PR fixes:
Fixes #
Bug evidence (required for bug-related PRs):
N/A
Special notes for your reviewer:
KTHENA_BATCH_FILES_DIR; with it unset,/v1/filesis not enabledpurpose=batch/v1/batchesAPI and in-router async workerDoes this PR introduce a user-facing change?: