-
Notifications
You must be signed in to change notification settings - Fork 250
Add files API #1893
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
base: main
Are you sure you want to change the base?
Add files API #1893
Conversation
# Conflicts: # packages/api/internal/api/api.gen.go
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 427d2ec5d6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| func (a *APIStore) GetVolumesVolumeIDDir(c *gin.Context, volumeID api.VolumeID, params api.GetVolumesVolumeIDDirParams) { | ||
| //TODO implement me | ||
| panic("implement me") |
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.
Implement volume content handlers instead of panicking
This endpoint is now reachable through the generated router, but the handler immediately calls panic("implement me"); the same placeholder is present in the other new /volumes/{volumeID}/dir and /volumes/{volumeID}/file handlers, so any request to the newly published files API fails at runtime with a 500 rather than performing the operation. Because these routes were added in this commit, this makes the feature non-functional as shipped.
Useful? React with 👍 / 👎.
| "500": | ||
| $ref: "#/components/responses/500" | ||
| post: | ||
| description: List directory contents |
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.
incorrect
| delete: | ||
| description: Delete a directory | ||
| tags: [volume_content] | ||
| security: | ||
| - AccessTokenAuth: [] | ||
| - ApiKeyAuth: [] | ||
| - Supabase1TokenAuth: [] | ||
| Supabase2TeamAuth: [] | ||
| parameters: | ||
| - $ref: "#/components/parameters/volumeID" | ||
| - $ref: "#/components/parameters/path" | ||
| responses: | ||
| "204": | ||
| description: "Successfully deleted a directory" | ||
| "404": | ||
| description: "path not found" | ||
| "500": | ||
| $ref: "#/components/responses/500" |
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.
I would just want delete both using /volumes/{volumeID}/file
Note
Medium Risk
Introduces new API surface for reading/writing/deleting volume contents and updates orchestrator gRPC contracts; current HTTP handlers are stubbed with
panic, so calling the new endpoints would crash until implemented.Overview
Adds a new volume content API that lets clients list/create/delete directories and upload/download/delete files within a volume via
/volumes/{volumeID}/dirand/volumes/{volumeID}/file, with a requiredpathquery parameter and a directory listing response schema. This updates the generated OpenAPI client/server code and the orchestratorVolumeServicegRPC protobuf to include matching directory/file RPCs and message types, and wires new server routes/handler method signatures (currently stubbed).Written by Cursor Bugbot for commit 967fa3e. This will update automatically on new commits. Configure here.