-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: improve storage structure documentation and implementation #20
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
Conversation
This commit improves the documentation and implementation of the storage structure in ChronDB: 1. Adds detailed documentation about the storage structure in README.md: - Explains how tables are represented as directories - Documents how files are organized within table directories - Provides examples of file paths with and without data directory 2. Updates git.clj implementation to better handle table-based storage: - Improves path handling for table-based document storage - Ensures proper encoding of table names and document IDs - Maintains backward compatibility with existing document paths The changes make the storage structure more explicit and easier to understand while maintaining compatibility with existing data. Signed-off-by: Avelino <[email protected]>
📝 WalkthroughWalkthroughThis pull request updates ChronDB’s storage system. The README now includes a new "Storage Structure" section that explains the organization of documents within table directories as JSON files, using document IDs as filenames. In the source code, the Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Storage as ChronDB Storage
participant Git as Git Repository
App->>Storage: Call save-document(document)
Storage->>Storage: Extract table name from document
Storage->>Storage: Call get-file-path(data-dir, id, table-name)
Storage->>Git: Write JSON document at constructed path
Storage-->>App: Confirm document saved
sequenceDiagram
participant App as Application
participant Storage as ChronDB Storage
participant Git as Git Repository
App->>Storage: Call get-document(id)
Storage->>Storage: Invoke get-document-path(repository, id)
Storage->>Git: Traverse directories to locate JSON file
Git-->>Storage: Return file path or nil
Storage-->>App: Return retrieved document or null
Possibly related PRs
Poem
🪧 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 (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
README.md (3)
45-45: Consider using more descriptive verbs in the documentation.
Stating "Documents are stored in a directory..." could be made more assertive or direct by using "organized" or "maintained." This is purely stylistic, but may read more smoothly.🧰 Tools
🪛 LanguageTool
[style] ~45-~45: Consider using a different verb to strengthen your wording.
Context: ...re stored in a directory structure that follows these rules: - Each table is a directo...(FOLLOW_OBEY)
54-56: Specify a language for your fenced code block.
According to markdownlint (MD040), it's recommended to include a language identifier after the triple backticks.Here's a possible fix:
-``` +```plaintext user/123.json🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
54-54: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
60-62: Add a language identifier to the fenced code block.
Similarly, consider specifyingplaintexthere for clarity and consistency.Here's a possible fix:
-``` +```plaintext {data-dir}/user/123.json🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
60-60: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
src/chrondb/storage/git.clj (1)
241-266: Potential improvement in searching for the document path.
Using.contains(path, (str encoded-id ".json"))works, but might be more explicit with something like.endsWith(path, (str encoded-id ".json"))to avoid any edge-case collisions. Otherwise, the approach is straightforward and likely sufficient for typical usage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)src/chrondb/storage/git.clj(7 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~45-~45: Consider using a different verb to strengthen your wording.
Context: ...re stored in a directory structure that follows these rules: - Each table is a directo...
(FOLLOW_OBEY)
🪛 markdownlint-cli2 (0.17.2)
README.md
54-54: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
60-60: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (5)
src/chrondb/storage/git.clj (5)
222-230: Good approach for backward compatibility with old ID format.
This fallback to a "default" table is helpful and ensures existing documents remain accessible. However, consider verifying how the code handles IDs with multiple colons (e.g.,"foo:bar:baz"). If needed, add tests or a note in the documentation to clarify expected behavior.
231-239: Solid implementation of the table-based file path.
This is a clear and maintainable approach to building consistent file paths.
393-405: Clean approach to table-based document filtering.
Logging the table path prefix helps with debugging, and checking for.jsonextension plus.startsWith(path, table-path-prefix + "/")ensures robust filtering.Also applies to: 417-419
436-480: Improved delete-document flow with table identification logic.
Fetching the document first to retrieve its_tablegreatly improves correctness and consistency with the rest of the storage approach.
496-496: Constructing GitStorage with a custom data directory is a nice enhancement.
This addition aligns well with the new table-based structure.
This commit improves the documentation and implementation of the storage structure in ChronDB:
Adds detailed documentation about the storage structure in README.md:
Updates git.clj implementation to better handle table-based storage:
The changes make the storage structure more explicit and easier to understand while maintaining compatibility with existing data.
Summary by CodeRabbit