支持可灵使用openai sdk生成视频#2004
Conversation
WalkthroughAdds JSON support for video submission parsing, introduces OpenAIVideo/OpenAIVideoError types, defines an OpenAIVideoConverter interface, implements converters for Sora and Kling adaptors, and updates /v1/videos/{id} fetch to use adaptor-based conversion instead of raw task data. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Middleware as Middleware
participant Validator as Validation (relay/common)
participant TaskSvc as Task Service
rect rgb(245,245,255)
note over Client,Middleware: POST /v1/videos
Client->>Middleware: Request (Content-Type)
alt Content-Type = multipart/form-data
Middleware->>Validator: Parse multipart form
else Content-Type = application/json
Middleware->>Validator: Unmarshal JSON body
end
Validator-->>Middleware: prompt, model, hasInputReference, action
end
Middleware-->>Client: Continue processing (not shown)
sequenceDiagram
autonumber
actor Client
participant Relay as relay_task.go
participant Resolver as GetTaskAdaptor
participant Adaptor as Channel Adaptor
participant Converter as OpenAIVideoConverter
rect rgb(245,255,245)
note over Client,Relay: GET /v1/videos/{id}
Client->>Relay: Fetch by ID
Relay->>Resolver: Resolve adaptor by platform
Resolver-->>Relay: adaptor
alt Adaptor implements OpenAIVideoConverter
Relay->>Converter: ConvertToOpenAIVideo(task)
Converter-->>Relay: OpenAIVideo or error
alt success
Relay-->>Client: 200 JSON(OpenAIVideo)
else error
Relay-->>Client: convert_to_openai_video_failed
end
else not implemented
Relay-->>Client: not_implemented
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
relay/channel/task/kling/adaptor.go (2)
373-380: Consider handling progress parsing errors more explicitly.The
convertProgressfunction returns0when parsing fails, which is indistinguishable from legitimate 0% progress. While logging a warning is good, consider whether the caller should be notified of parse failures.If you want to distinguish between valid 0% and parse errors, consider returning a default like -1 or wrapping in an optional type:
convertProgress := func(progress string) int { progress = strings.TrimSuffix(progress, "%") p, err := strconv.Atoi(progress) if err != nil { logger.Warnf("convert progress failed, progress: %s, err: %v", progress, err) + return -1 // or handle as a special case } return p }
404-409: Reconsider the error attachment condition.The check
klingResp.Code != 0 && klingResp.Message != ""may miss error cases where:
- Code is 0 but Message indicates an error
- Code is non-zero but Message is empty
Consider a more robust check:
- if klingResp.Code != 0 && klingResp.Message != "" { + if klingResp.Code != 0 || klingResp.Message != "" { openAIVideo.Error = &relaycommon.OpenAIVideoError{ Message: klingResp.Message, Code: fmt.Sprintf("%d", klingResp.Code), } }Or if Code == 0 always means success:
if klingResp.Code != 0 && klingResp.Message != "" { openAIVideo.Error = &relaycommon.OpenAIVideoError{ - Message: klingResp.Message, + Message: lo.Ternary(klingResp.Message != "", klingResp.Message, "Unknown error"), Code: fmt.Sprintf("%d", klingResp.Code), } }relay/common/relay_utils.go (1)
108-161: Consider refactoring to reduce complexity.The function now handles two different content types with branching logic, which increases cognitive complexity. While functional, consider extracting the parsing logic into separate helper functions for better maintainability.
Example refactor:
func parseMultipartVideo(c *gin.Context) (prompt string, hasInputReference bool, err *dto.TaskError) { // ... multipart parsing logic } func parseJSONVideo(c *gin.Context) (prompt string, hasInputReference bool, err *dto.TaskError) { // ... JSON parsing logic } func ValidateMultipartDirect(c *gin.Context, info *RelayInfo) *dto.TaskError { contentType := c.GetHeader("Content-Type") var prompt string var hasInputReference bool var taskErr *dto.TaskError if strings.HasPrefix(contentType, "multipart/form-data") { prompt, hasInputReference, taskErr = parseMultipartVideo(c) } else { prompt, hasInputReference, taskErr = parseJSONVideo(c) } if taskErr != nil { return taskErr } // ... rest of validation }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
middleware/distributor.go(1 hunks)relay/channel/adapter.go(2 hunks)relay/channel/task/kling/adaptor.go(2 hunks)relay/channel/task/sora/adaptor.go(1 hunks)relay/common/relay_info.go(1 hunks)relay/common/relay_utils.go(1 hunks)relay/relay_task.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
relay/channel/adapter.go (2)
model/task.go (3)
Task(23-42)Task(231-235)Task(237-241)relay/common/relay_info.go (1)
OpenAIVideo(554-569)
middleware/distributor.go (1)
common/gin.go (2)
ParseMultipartFormReusable(118-139)UnmarshalBodyReusable(31-52)
relay/channel/task/sora/adaptor.go (2)
relay/common/relay_info.go (1)
OpenAIVideo(554-569)common/json.go (1)
Unmarshal(8-10)
relay/common/relay_utils.go (2)
common/gin.go (2)
ParseMultipartFormReusable(118-139)UnmarshalBodyReusable(31-52)relay/common/relay_info.go (1)
TaskSubmitReq(484-493)
relay/channel/task/kling/adaptor.go (3)
relay/channel/adapter.go (1)
TaskAdaptor(33-52)model/task.go (3)
Task(23-42)Task(231-235)Task(237-241)relay/common/relay_info.go (2)
OpenAIVideo(554-569)OpenAIVideoError(570-573)
relay/relay_task.go (3)
relay/relay_adaptor.go (1)
GetTaskAdaptor(122-146)service/error.go (2)
TaskErrorWrapperLocal(133-137)TaskErrorWrapper(139-155)relay/channel/adapter.go (1)
OpenAIVideoConverter(54-56)
🔇 Additional comments (5)
relay/channel/adapter.go (1)
54-56: LGTM!The interface is well-defined with a clear contract for converting task data to OpenAI video format.
relay/channel/task/sora/adaptor.go (1)
188-195: LGTM! Implementation correctly unmarshals stored OpenAIVideo data.The implementation is clean and properly handles errors. The approach of unmarshaling task.Data directly assumes it was stored in OpenAIVideo format during task creation.
relay/relay_task.go (1)
376-384: LGTM! Proper use of type assertion and converter interface.The type assertion with
okcheck and the conversion flow are implemented correctly.relay/common/relay_info.go (1)
554-573: LGTM! Well-structured OpenAI video types.The data structures are well-defined with appropriate JSON tags. The Progress field as
intis clear, though note that conversion from string progress (e.g., "75%") must be handled by the converters.middleware/distributor.go (1)
177-194: Potential index out of bounds when accessing form.Value["model"].Line 186 accesses
form.Value["model"][0]without first checking if the slice has elements, which could cause a panic if the model field is present but empty.Apply this diff to add a length check:
if form != nil { - if values, ok := form.Value["model"]; ok && len(values) > 0 { + if values, ok := form.Value["model"]; ok && len(values) > 0 && values[0] != "" { modelRequest.Model = values[0] } }Or alternatively, validate that the model value is non-empty:
if form != nil { if values, ok := form.Value["model"]; ok && len(values) > 0 { - modelRequest.Model = values[0] + if values[0] != "" { + modelRequest.Model = values[0] + } } }Likely an incorrect or invalid review comment.
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/bytedance/gopkg/util/logger" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify the new external logger dependency.
The import of github.com/bytedance/gopkg/util/logger introduces a new external dependency. Ensure this aligns with the project's dependency management strategy.
Run the following script to check if this logger is used elsewhere in the codebase:
🏁 Script executed:
#!/bin/bash
# Check usage of bytedance logger across the codebase
rg -n "github.com/bytedance/gopkg/util/logger" --type=goLength of output: 80
Address logger dependency and error handling
- Confirm import of
github.com/bytedance/gopkg/util/loggeraligns with project policy; it’s only used in adaptor.go. - convertProgress returns 0 on parse failure without signaling an error—return an error or log failures to avoid silent zero progress.
- Error attachment check
if resp.Code != 0 && resp.Message != ""misses message-only errors—use||instead.
🤖 Prompt for AI Agents
In relay/channel/task/kling/adaptor.go around line 14, ensure the import of
github.com/bytedance/gopkg/util/logger conforms to our dependency policy (either
keep it if allowed for adaptor.go only or replace with the project's approved
logger package and update call sites), change convertProgress so it does not
silently return 0 on parse failures by returning an (int, error) or logging the
parse failure and propagating the error to callers, and update the
error-attachment check from using && to || (i.e., if resp.Code != 0 ||
resp.Message != "") so message-only errors are not missed; adjust callers to
handle the new convertProgress error return or rely on the logged error.
| if _, ok := form.Value["model"]; !ok { | ||
| return createTaskError(fmt.Errorf("model field is required"), "missing_model", http.StatusBadRequest, true) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate that model value is non-empty.
The code checks if the "model" field exists in the form but doesn't validate that it's not an empty string, which could lead to downstream validation issues.
Apply this diff:
- if _, ok := form.Value["model"]; !ok {
+ models, hasModel := form.Value["model"]
+ if !hasModel || len(models) == 0 || strings.TrimSpace(models[0]) == "" {
return createTaskError(fmt.Errorf("model field is required"), "missing_model", http.StatusBadRequest, true)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if _, ok := form.Value["model"]; !ok { | |
| return createTaskError(fmt.Errorf("model field is required"), "missing_model", http.StatusBadRequest, true) | |
| } | |
| models, hasModel := form.Value["model"] | |
| if !hasModel || len(models) == 0 || strings.TrimSpace(models[0]) == "" { | |
| return createTaskError(fmt.Errorf("model field is required"), "missing_model", http.StatusBadRequest, true) | |
| } |
🤖 Prompt for AI Agents
In relay/common/relay_utils.go around lines 126 to 128, the code only checks
that the "model" key exists but not that its value is non-empty; update the
validation to ensure form.Value["model"] has at least one entry and that the
first entry is not an empty or whitespace-only string (e.g., check
len(valueSlice) > 0 and strings.TrimSpace(valueSlice[0]) != ""), and return the
same createTaskError("model field is required", "missing_model",
http.StatusBadRequest, true) when the value is empty; import strings if needed.
| adaptor := GetTaskAdaptor(originTask.Platform) | ||
| if adaptor == nil { | ||
| taskResp = service.TaskErrorWrapperLocal(fmt.Errorf("invalid channel id: %d", originTask.ChannelId), "invalid_channel_id", http.StatusBadRequest) | ||
| return |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve error message accuracy.
The error code "invalid_channel_id" on Line 373 is misleading when the actual issue is that no adaptor exists for the platform. Consider using a more descriptive error code.
Apply this diff:
adaptor := GetTaskAdaptor(originTask.Platform)
if adaptor == nil {
- taskResp = service.TaskErrorWrapperLocal(fmt.Errorf("invalid channel id: %d", originTask.ChannelId), "invalid_channel_id", http.StatusBadRequest)
+ taskResp = service.TaskErrorWrapperLocal(fmt.Errorf("no adaptor found for platform: %s", originTask.Platform), "adaptor_not_found", http.StatusBadRequest)
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| adaptor := GetTaskAdaptor(originTask.Platform) | |
| if adaptor == nil { | |
| taskResp = service.TaskErrorWrapperLocal(fmt.Errorf("invalid channel id: %d", originTask.ChannelId), "invalid_channel_id", http.StatusBadRequest) | |
| return | |
| adaptor := GetTaskAdaptor(originTask.Platform) | |
| if adaptor == nil { | |
| taskResp = service.TaskErrorWrapperLocal(fmt.Errorf("no adaptor found for platform: %s", originTask.Platform), "adaptor_not_found", http.StatusBadRequest) | |
| return | |
| } |
🤖 Prompt for AI Agents
In relay/relay_task.go around lines 371 to 374, the error returned when
GetTaskAdaptor(originTask.Platform) is nil currently reports "invalid channel
id" and uses error code "invalid_channel_id", which is misleading; change the
error to reference the missing/unsupported platform (e.g., fmt.Errorf("no
adaptor for platform: %v", originTask.Platform)) and use a descriptive error
code like "unsupported_platform" (retain http.StatusBadRequest), so the log and
response accurately reflect that no adaptor exists for the given platform.
支持可灵使用openai sdk生成视频
2 participants
import OpenAI from 'openai';视频生成:
await client.videos.create(requestParams);视频查询:
await client.videos.retrieve(taskId);Summary by CodeRabbit