Repository navigation
Remove deprecated CSV/JSON storage options - #214
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughCSV/JSON storage backends were removed across core, server CLI, TUI, tests, and documentation. Features disabling JSON storage were updated. Entry events, constructors, enum variants, UI menu items, help text, and tests were adjusted accordingly. Proxy remains available and is repositioned in the TUI. Git, File, Memory, and Mongo backends persist. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant TUI as TUI Entry Menu
participant Core as Core State
participant DB as Db Builder
participant BE as Backend
User->>TUI: Select backend (Memory/File/Git/Mongo/Proxy)
TUI->>Core: EntryEvent(OpenMemory|OpenFile|OpenGit|OpenMongo|OpenProxy)
Core->>DB: Build corresponding Db
alt Local backends (Memory/File/Git/Mongo)
DB->>BE: Initialize backend
BE-->>DB: Ready
else Proxy
DB->>BE: Connect to remote proxy
BE-->>DB: Ready
end
DB-->>Core: Db handle
Core-->>TUI: OpenNotebook
note over TUI,Core: CSV/JSON flows removed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (12)
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 |
Glues Coverage Report
|
![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 (7)
README.md (2)
10-15: Clarify supported backends in intro to match Usage section.Line 14 says “Currently, it supports Git and MongoDB…”, but the app also supports Instant (memory), Local (files), and Proxy. Suggest rewording for consistency.
-Currently, it supports Git and MongoDB for storage, and we plan to integrate additional storage options through [GlueSQL](https://github.com/gluesql/gluesql), giving you even more flexibility in managing your data. +Currently, it supports Instant (in‑memory), Local (files), Git, and MongoDB for storage, and we plan to integrate additional storage options through [GlueSQL](https://github.com/gluesql/gluesql), giving you even more flexibility in managing your data. You can also connect via a Proxy to a remote backend.
53-54: Fix markdownlint MD004: unify list marker style.Your config expects asterisks; these two sub-bullets use dashes.
- - Run `cargo run -p glues-proxy-server -- memory` (or file/git/mongo) to start a local proxy. - - In the TUI entry menu choose `[5] Proxy`, enter the proxy URL (e.g. `http://127.0.0.1:4000`), and Glues will talk to the remote backend just like it does locally. + * Run `cargo run -p glues-proxy-server -- memory` (or file/git/mongo) to start a local proxy. + * In the TUI entry menu choose `[5] Proxy`, enter the proxy URL (e.g. `http://127.0.0.1:4000`), and Glues will talk to the remote backend just like it does locally.core/src/backend/local.rs (1)
92-119: Consider signaling misuse of pull/sync on non‑Git backends.Currently no‑ops; optionally return a typed error or log a notice to aid callers.
- pub fn pull(&mut self) -> Result<()> { - if let Storage::Git(glue) = &mut self.storage { - glue.storage.pull()?; - } - Ok(()) - } + pub fn pull(&mut self) -> Result<()> { + match &mut self.storage { + Storage::Git(glue) => glue.storage.pull()?, + _ => { + log::debug!("Db::pull called on non-Git storage (no-op)"); + } + } + Ok(()) + }tui/src/views/dialog/help.rs (1)
70-74: Mirror README note about default port/--listen for parity.Small UX boost: add “listens on 127.0.0.1:4000 by default; use --listen to change”.
-Run `cargo run -p glues-proxy-server -- memory` (or file/git/mongo) to start a local proxy. +Run `cargo run -p glues-proxy-server -- memory` (or file/git/mongo) to start a local proxy (defaults to 127.0.0.1:4000; use `--listen` to change).tui/src/action.rs (3)
201-215: Guard against empty/invalid Proxy URL before dispatch.Provide immediate feedback instead of failing downstream.
Action::Tui(TuiAction::OpenProxy) => { let url = self .context .take_prompt_input() .log_expect("proxy url must not be none"); - - config::update(LAST_PROXY_URL, &url).await; + let url = url.trim().to_string(); + if url.is_empty() { + self.context.alert = Some("Proxy URL cannot be empty".to_string()); + return false; + } + if !(url.starts_with("http://") || url.starts_with("https://")) { + self.context.alert = Some("Proxy URL must start with http:// or https://".to_string()); + return false; + } + config::update(LAST_PROXY_URL, &url).await;
186-201: Validate Mongo database name before dispatch.Consistent with file path validation; avoids trivial backend errors.
Action::Tui(TuiAction::OpenMongo(OpenMongoStep::Database { conn_str })) => { let db_name = self .context .take_prompt_input() .log_expect("database name must not be none"); + if db_name.trim().is_empty() { + self.context.alert = Some("Database name cannot be empty".to_string()); + return false; + } config::update(LAST_MONGO_DB_NAME, &db_name).await;
150-168: Optional: validate Git branch before dispatch.Prevents immediate error if left blank.
Action::Tui(TuiAction::OpenGit(OpenGitStep::Branch { path, remote })) => { let branch = self .context .take_prompt_input() .log_expect("branch must not be none"); + if branch.trim().is_empty() { + self.context.alert = Some("Git branch cannot be empty".to_string()); + return false; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.locktui/tests/snapshots/entry__csv_alert.snapis excluded by!**/*.snaptui/tests/snapshots/entry__help_closed.snapis excluded by!**/*.snaptui/tests/snapshots/entry__help_open.snapis excluded by!**/*.snaptui/tests/snapshots/entry__local_prompt.snapis excluded by!**/*.snaptui/tests/snapshots/entry__proxy_prompt_open.snapis excluded by!**/*.snap
📒 Files selected for processing (12)
Cargo.toml(0 hunks)README.md(3 hunks)core/src/backend/local.rs(1 hunks)core/src/event.rs(0 hunks)core/src/state/entry.rs(0 hunks)server/src/main.rs(0 hunks)tui/Cargo.toml(1 hunks)tui/src/action.rs(1 hunks)tui/src/context/entry.rs(3 hunks)tui/src/views/body/entry.rs(1 hunks)tui/src/views/dialog/help.rs(1 hunks)tui/tests/entry.rs(1 hunks)
💤 Files with no reviewable changes (4)
- Cargo.toml
- core/src/state/entry.rs
- core/src/event.rs
- server/src/main.rs
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
53-53: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
54-54: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: coverage
🔇 Additional comments (8)
tui/Cargo.toml (2)
8-8: Description update aligns with feature set (LGTM).Accurately reflects removal of CSV/JSON and promotion of Proxy.
1-32: Sweep for stale CSV/JSON mentions — confirm which to keep/removeFound occurrences:
- server/src/main.rs — Json import and JSON handler types (lines 3, 114–118).
- core/src/backend/proxy/client.rs — reqwest .json() calls (lines 22–23, 40–41).
- core/tests/proxy.rs — "application/json" test header (line 37).
- core/Cargo.toml — reqwest features include "json" (line 18).
- Cargo.toml — "gluesql-csv-storage" in workspace (line 23).
- Cargo.lock — csv, csv-core, gluesql-csv-storage, gluesql-json-storage entries.
- AGENTS.md — JSON-related instruction (line 13).
- deny.toml — commented deny = ["json"] (line 183).
If CSV storage is obsolete: remove gluesql-csv-storage from Cargo.toml, update/remove docs (AGENTS.md/README), and regenerate Cargo.lock. If JSON usage is intentional (server/reqwest/tests), keep as-is and close this sweep.
core/src/backend/local.rs (1)
13-14: Removed CSV/JSON imports and variants (LGTM).Imports and enum now match the reduced backend matrix. Constructors and dispatch look consistent.
tui/src/views/body/entry.rs (1)
35-41: Uniform menu styling after CSV/JSON removal (LGTM).Simpler and consistent; no functional risk.
tui/src/views/dialog/help.rs (1)
70-74: Help text update for Proxy (LGTM).Matches the new menu index and workflow.
tui/src/context/entry.rs (2)
16-21: Menu renumbering and size update (LGTM).PROXY -> [5], MENU_ITEMS -> 7 entries; consistent with UI and docs.
95-100: Keybinding for Proxy (LGTM).Direct hotkey mapping added for '5'.
tui/tests/entry.rs (1)
70-79: Test migration from CSV to Proxy prompt — LGTM; confirm insta snapshot updateHotkey [5] and snapshot rename align with UI changes (tui/tests/entry.rs:70-79). Confirm the insta snapshot "proxy_prompt_open" exists and was updated in this PR. Run:
rg -n "proxy_prompt_open" tui/tests -S rg -n "proxy_prompt_open" tui/tests/snapshots -S git status --porcelain | rg 'tui/tests/snapshots' || true
Summary
Testing
Summary by CodeRabbit