fix(wren-ui): delete semantics when reseting project#1490
Conversation
WalkthroughThe changes introduce three key modifications. First, a new entry is added to the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant R as ProjectResolver
participant A as WrenAIAdaptor
participant S as RemoteService
C->>R: Request resetProject(id)
R->>R: Delete project-related entities (views, models, etc.)
R->>A: Call delete(id)
A->>S: HTTP DELETE /v1/semantics?projectId=id
S-->>A: Respond (200 or error)
A-->>R: Return result
R-->>C: Return final response
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts (1)
131-153: Implementation handles errors properly but could improve validationThe implementation of the
deletemethod properly makes an HTTP DELETE request to the semantics endpoint and includes good error handling. However, there are a few opportunities for improvement:Consider these improvements:
- Add more specific input validation for
projectId(checking if it's a positive number)- Log errors at the appropriate level (using
logger.errorinstead of only throwing)- Consider adding more specific error handling for different HTTP status codes
public async delete(projectId: number): Promise<void> { try { if (!projectId) { throw new Error('Project ID is required'); + } else if (!Number.isInteger(projectId) || projectId <= 0) { + throw new Error('Project ID must be a positive integer'); } const url = `${this.wrenAIBaseEndpoint}/v1/semantics`; const response = await axios.delete(url, { params: { project_id: projectId.toString(), }, }); if (response.status === 200) { logger.info(`Wren AI: Deleted semantics for project ${projectId}`); + } else if (response.status === 404) { + logger.warn(`Wren AI: No semantics found for project ${projectId}`); + // Not throwing an error since this is an acceptable state } else { + logger.error(`Wren AI: Failed to delete semantics for project ${projectId}. Status: ${response.status}, Error: ${response.data?.error}`); throw new Error(`Failed to delete semantics. ${response.data?.error}`); } } catch (error: any) { + logger.error(`Wren AI: Error deleting semantics for project ${projectId}:`, error); throw new Error( `Wren AI: Failed to delete semantics: ${getAIServiceError(error)}`, ); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts(2 hunks)wren-ui/src/apollo/server/resolvers/projectResolver.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts (1)
wren-ui/src/apollo/client/graphql/__types__.ts (1)
Error(433-439)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
.gitignore (1)
32-32: Added appropriate entry to .gitignoreThe addition of
docker/docker-compose-local.yamlto the gitignore file is appropriate as it prevents committing local Docker configuration, which typically contains environment-specific settings.wren-ui/src/apollo/server/resolvers/projectResolver.ts (1)
133-133: Properly integrates semantics cleanup in project resetThe addition of the
deletecall to clean up semantics is well-placed after all other project-related entities are deleted, ensuring proper cleanup order. This implementation fixes the issue mentioned in the PR title about deleting semantics when resetting a project.wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts (1)
51-51: LGTM: Interface method correctly declaredThe addition of the
deletemethod to theIWrenAIAdaptorinterface is correctly defined with appropriate parameter and return type.
2 participants
delete semantics when reseting project
Summary by CodeRabbit
Chores
New Features