Option to configure vector and log storages using UI #486
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughBackend: adds GET/PUT endpoints for vector-store and log-store configs and simplifies generic updateConfig response. UI: adds VectorStoreForm and LogStoreForm components, RTK Query endpoints/tags, and TypeScript types for vector and log store configurations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Config UI (VectorStoreForm)
participant API as HTTP API (/api/config/vector-store)
participant Store as ConfigStore
User->>UI: Open Config page
UI->>API: GET /api/config/vector-store
API->>Store: GetVectorStoreConfig()
alt store available
Store-->>API: VectorStoreConfig
API-->>UI: 200 OK + config
else store nil
API-->>UI: 503 Service Unavailable
end
User->>UI: Edit settings + Save
UI->>API: PUT /api/config/vector-store (payload)
API->>Store: UpdateVectorStoreConfig(payload)
alt success
API-->>UI: 200 OK {"status":"success"}
else error
API-->>UI: 500 Internal Server Error
end
sequenceDiagram
autonumber
actor User
participant UI as Config UI (LogStoreForm)
participant API as HTTP API (/api/config/log-store)
participant Store as ConfigStore
User->>UI: Open Config page
UI->>API: GET /api/config/log-store
API->>Store: GetLogsStoreConfig()
alt store available
Store-->>API: LogStoreConfig
API-->>UI: 200 OK + config
else store nil
API-->>UI: 503 Service Unavailable
end
User->>UI: Edit settings + Save
UI->>API: PUT /api/config/log-store (payload)
API->>Store: UpdateLogsStoreConfig(payload)
alt success
API-->>UI: 200 OK {"status":"success"}
else error
API-->>UI: 500 Internal Server Error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 2
🧹 Nitpick comments (13)
transports/bifrost-http/handlers/config.go (2)
159-162: Use 503 for unavailable config store (consistency with GET).GET handlers return 503 when the config store is nil; PUT handlers return 500. Align PUT to 503 to reflect service unavailability and keep semantics consistent.
Apply:
- SendError(ctx, fasthttp.StatusInternalServerError, "Config store not initialized", h.logger) + SendError(ctx, fasthttp.StatusServiceUnavailable, "config store not available", h.logger)Also applies to: 200-203
137-138: Redundant status set before SendJSON.SendJSON already sets 200. You can drop the explicit SetStatusCode for consistency.
- ctx.SetStatusCode(fasthttp.StatusOK) SendJSON(ctx, map[string]string{"status": "success"}, h.logger)ui/lib/types/config.ts (1)
274-278: Weaviate advanced fields absent (optional).Consider exposing optional fields present in backend (headers, grpc_config, timeout) to keep parity.
ui/app/config/page.tsx (1)
369-372: Gate store forms when DB/config-store is unavailable.If ConfigStore is down, the forms render but cannot save (backend returns 503). Show a clear message or hide sections until is_db_connected is true.
- <VectorStoreForm /> - - <LogStoreForm /> + {bifrostConfig?.is_db_connected ? ( + <> + <VectorStoreForm /> + <LogStoreForm /> + </> + ) : ( + <Alert> + <AlertDescription>Config store is unavailable. Connect the database to configure Vector/Log store.</AlertDescription> + </Alert> + )}ui/app/config/views/logStoreForm.tsx (2)
21-29: Handle GET error state from RTK Query.If the API returns 503, the form renders defaults with no feedback. Show an error and avoid edits.
- const { data: logStoreConfig, isLoading } = useGetLogStoreConfigQuery(); + const { data: logStoreConfig, isLoading, error } = useGetLogStoreConfigQuery();- if (isLoading) { + if (isLoading) { return <div>Loading log store configuration...</div>; - } + } + if (error) { + return ( + <Alert variant="destructive"> + <AlertDescription>{getErrorMessage(error)}</AlertDescription> + </Alert> + ); + }
135-141: Basic validation: require non-empty path when enabled.Prevent saving invalid config and reduce backend churn.
- <Button onClick={handleSave} className="flex items-center gap-2"> + <Button + onClick={handleSave} + disabled={localConfig.enabled && !((localConfig.config as SQLiteConfig).path || "").trim()} + className="flex items-center gap-2" + >ui/app/config/views/vectorStoreForm.tsx (4)
229-237: onValueChange typing: cast to VectorStoreType to satisfy TS and prevent leakage of arbitrary strings.ShadCN Select provides string; narrow before use.
- <Select value={localConfig.type} onValueChange={handleTypeChange}> + <Select value={localConfig.type} onValueChange={(v) => handleTypeChange(v as VectorStoreType)}>
161-164: Explicit radix for parseInt.Avoids edge-cases with leading zeros.
- onChange={(e) => handleRedisConfigChange("db", parseInt(e.target.value) || 0)} + onChange={(e) => handleRedisConfigChange("db", Number.parseInt(e.target.value, 10) || 0)}
32-40: Surface GET error state.Mirror LogStoreForm recommendation to show API errors instead of silently rendering defaults.
- const { data: vectorStoreConfig, isLoading } = useGetVectorStoreConfigQuery(); + const { data: vectorStoreConfig, isLoading, error } = useGetVectorStoreConfigQuery();- if (isLoading) { + if (isLoading) { return <div>Loading vector store configuration...</div>; - } + } + if (error) { + return ( + <Alert variant="destructive"> + <AlertDescription>{getErrorMessage(error)}</AlertDescription> + </Alert> + ); + }
256-263: Disable Save on invalid required fields.When enabled: require host for Weaviate or addr for Redis before allowing save.
- <Button onClick={handleSave} className="flex items-center gap-2"> + {(() => { + const cfg = localConfig.config as WeaviateConfig | RedisConfig; + const isValid = + !localConfig.enabled || + (localConfig.type === "weaviate" ? !!(cfg as WeaviateConfig).host?.trim() : !!(cfg as RedisConfig).addr?.trim()); + return ( + <Button onClick={handleSave} disabled={!isValid} className="flex items-center gap-2"> + <Save className="h-4 w-4" /> + Save Configuration + </Button> + ); + })()}ui/lib/store/apis/configApi.ts (3)
41-48: Use void for mutation response; optionally add optimistic cache updatefetchBaseQuery returns undefined for 204/empty responses. Prefer
voidovernull. Optionally apply optimistic cache update to keep forms snappy.- updateVectorStoreConfig: builder.mutation<null, VectorStoreConfig>({ + updateVectorStoreConfig: builder.mutation<void, VectorStoreConfig>({ query: (data) => ({ url: "/config/vector-store", method: "PUT", body: data, }), + async onQueryStarted(data, { dispatch, queryFulfilled }) { + const patch = dispatch( + baseApi.util.updateQueryData('getVectorStoreConfig', undefined, (draft) => { + Object.assign(draft, data); + }) + ); + try { + await queryFulfilled; + } catch { + patch.undo(); + } + }, invalidatesTags: ["VectorStoreConfig", "Config"], }),
59-66: Use void for mutation response; mirror optimistic update (optional)Align return type with empty/204 responses. Optional optimistic update shown below.
- updateLogStoreConfig: builder.mutation<null, LogStoreConfig>({ + updateLogStoreConfig: builder.mutation<void, LogStoreConfig>({ query: (data) => ({ url: "/config/log-store", method: "PUT", body: data, }), + async onQueryStarted(data, { dispatch, queryFulfilled }) { + const patch = dispatch( + baseApi.util.updateQueryData('getLogStoreConfig', undefined, (draft) => { + Object.assign(draft, data); + }) + ); + try { + await queryFulfilled; + } catch { + patch.undo(); + } + }, invalidatesTags: ["LogStoreConfig", "Config"], }),
70-79: Export lazy query hooks for symmetryExpose lazy hooks for vector/log to match core config exports and enable on-demand fetches in forms.
export const { useGetVersionQuery, useGetCoreConfigQuery, useUpdateCoreConfigMutation, useLazyGetCoreConfigQuery, useGetVectorStoreConfigQuery, + useLazyGetVectorStoreConfigQuery, useUpdateVectorStoreConfigMutation, useGetLogStoreConfigQuery, + useLazyGetLogStoreConfigQuery, useUpdateLogStoreConfigMutation } = configApi;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
transports/bifrost-http/handlers/config.go(3 hunks)ui/app/config/page.tsx(2 hunks)ui/app/config/views/logStoreForm.tsx(1 hunks)ui/app/config/views/vectorStoreForm.tsx(1 hunks)ui/lib/store/apis/baseApi.ts(1 hunks)ui/lib/store/apis/configApi.ts(2 hunks)ui/lib/types/config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
ui/app/config/page.tsx (2)
ui/app/config/views/vectorStoreForm.tsx (1)
VectorStoreForm(31-269)ui/app/config/views/logStoreForm.tsx (1)
LogStoreForm(20-148)
ui/lib/types/config.ts (4)
framework/vectorstore/store.go (1)
VectorStoreType(12-12)framework/vectorstore/weaviate.go (1)
WeaviateConfig(25-37)framework/vectorstore/redis.go (1)
RedisConfig(22-40)framework/configstore/sqlite.go (1)
SQLiteConfig(37-39)
ui/app/config/views/logStoreForm.tsx (2)
ui/lib/types/config.ts (2)
SQLiteConfig(306-308)LogStoreConfig(310-314)ui/lib/store/apis/baseApi.ts (1)
getErrorMessage(87-109)
ui/app/config/views/vectorStoreForm.tsx (2)
ui/lib/types/config.ts (4)
WeaviateConfig(274-278)RedisConfig(280-295)VectorStoreConfig(297-301)VectorStoreType(272-272)ui/lib/store/apis/baseApi.ts (1)
getErrorMessage(87-109)
transports/bifrost-http/handlers/config.go (3)
transports/bifrost-http/handlers/utils.go (2)
SendJSON(16-24)SendError(27-36)framework/configstore/store.go (1)
ConfigStore(14-100)transports/bifrost-http/lib/config.go (1)
Config(113-136)
ui/lib/store/apis/configApi.ts (1)
ui/lib/types/config.ts (2)
VectorStoreConfig(297-301)LogStoreConfig(310-314)
🔇 Additional comments (4)
transports/bifrost-http/handlers/config.go (1)
43-50: New vector/log store routes: LGTM.Endpoints are coherent with existing /api/config surface.
ui/lib/store/apis/configApi.ts (3)
1-1: Imports and types: LGTMInterfaces align with endpoint generics; no unused imports.
50-56: Log store GET endpoint: LGTMConsistent with vector-store; correct types and tags.
32-38: Vector store GET endpoint: LGTM — verification incomplete; confirm TagTypes/types/backend routeRTK Query usage looks correct; repo search returned no matches for: tagTypes in baseApi.ts; VectorStoreConfig / LogStoreConfig in ui/lib/types/config.ts; or backend Go route "/config/vector-store". Provide or confirm the files/paths or add the missing definitions.
|
@Pratham-Mishra04 can you please review this and let me know if any changes are required |
|
Hey @rohhann12 thanks for the PR, a few minor changes are required rest looks good :) |
| if (logStoreConfig) { | ||
| const hasConfigChanges = JSON.stringify(localConfig) !== JSON.stringify(logStoreConfig); | ||
| setHasChanges(hasConfigChanges); | ||
| setNeedsRestart(hasConfigChanges); |
There was a problem hiding this comment.
won't this logic not show the alert when we make some change and the just reload the page without restart bifrost? as we are directly making update in the DB via the handlers
2 participants
Summary
Briefly explain the purpose of this PR and the problem it solves.
added api individual for logstore and vector store
updated ui
Changes
Type of change
Affected areas
How to test
Describe the steps to validate this change. Include commands and expected outcomes.
If adding new configs or environment variables, document them here.
Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
Breaking changes
If yes, describe impact and migration instructions.
Related issues
Link related issues and discussions. Example: Closes #123
Security considerations
Note any security implications (auth, secrets, PII, sandboxing, etc.).
Checklist
docs/contributing/README.mdand followed the guidelines