feat(vespa): Move vespa to standalone lib#1231
Conversation
junaid-shirur
requested review from
devesh-juspay,
kalpadhwaryu,
shivamashtikar and
zereraz
as code owners
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR removes Vespa deployment/configuration and most Vespa schema files, deletes several Vespa utility scripts, and updates Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Script as clear-vespa-data.ts
participant Vespa as VespaClient
note right of Script `#DDEEFF`: uses hardcoded schemas: my_content.*
Script->>Vespa: list documents for schema (GetDocument / search)
alt documents found
loop per-batch
Script->>Vespa: DeleteDocument (batch)
Vespa-->>Script: delete result
end
Script->>Script: log progress / handle errors
else no documents
Vespa-->>Script: empty result
end
Script-->>Vespa: optional UpdateDocument (permissions/metadata)
Vespa-->>Script: result
Note over Vespa,Script: All interactions use Vespa client calls (programmatic)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
💤 Files with no reviewable changes (3)
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 |
Summary of ChangesHello @junaid-shirur, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant refactoring of the Vespa search engine integration. By moving the Vespa configuration, schema definitions, and deployment logic into a separate, standalone library, the main application's codebase becomes cleaner and more modular. This change streamlines the project structure, reduces internal dependencies on Vespa-specific files, and prepares the system for more efficient management of its search infrastructure. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant refactoring that moves Vespa-related configurations, schemas, and scripts into a standalone library. This is a great step for modularity. The changes primarily involve the removal of numerous files from the server/vespa/ directory. My review focuses on the clear-vespa-data.ts script, which was updated to accommodate these changes. I've provided a suggestion to improve its maintainability by leveraging constants from the new library instead of hardcoded strings.
| async function getVespaSchemas(): Promise<string[]> { | ||
| // Returns prefixed names e.g. "my_content.file" | ||
| // Determine path relative to the current file's directory | ||
| const scriptDir = path.dirname(new URL(import.meta.url).pathname) | ||
| const schemasDir = path.join(scriptDir, "../vespa/schemas") | ||
| try { | ||
| const files = await fs.readdir(schemasDir) | ||
| const schemaNames = files | ||
| .filter((file) => file.endsWith(".sd")) | ||
| .map((file) => `my_content.${file.replace(".sd", "")}`) // No incorrect cast here | ||
| return schemaNames | ||
| } catch (error) { | ||
| console.error(`Error reading Vespa schemas directory ${schemasDir}:`, error) | ||
| return [] | ||
| } | ||
| const schemaNames = [ | ||
| "file", | ||
| "user", | ||
| "mail", | ||
| "mail_attachment", | ||
| "event", | ||
| "chat_message", | ||
| "chat_container", | ||
| "chat_user", | ||
| "chat_team", | ||
| "chat_attachment", | ||
| "datasource", | ||
| "datasource_file", | ||
| "kb_items", | ||
| "user_query", | ||
| ] | ||
| return schemaNames.map((name) => `my_content.${name}`) | ||
| } |
There was a problem hiding this comment.
To improve maintainability and avoid magic strings, it would be better to use the schema name constants that are already imported from @xyne/vespa-ts/types (e.g., fileSchema, userSchema).
This also reveals that some schemas (datasource, datasource_file, kb_items) are not imported as constants. If these are also defined in the new library, they should be imported and used as constants as well. This would make this list less brittle to future changes in the library.
async function getVespaSchemas(): Promise<string[]> {
const schemaNames = [
fileSchema,
userSchema,
mailSchema,
mailAttachmentSchema,
eventSchema,
chatMessageSchema,
chatContainerSchema,
chatUserSchema,
chatTeamSchema,
chatAttachmentSchema,
// The following are hardcoded. If constants for these are available
// from `@xyne/vespa-ts/types`, please import and use them for consistency.
"datasource",
"datasource_file",
"kb_items",
userQuerySchema,
];
return schemaNames.map((name) => `my_content.${name}`);
}
2 participants
Description
Testing
Additional Notes
Summary by CodeRabbit
Chores
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.