Consolidate & fix storage docs#142
Conversation
|
📝 WalkthroughWalkthroughThis PR updates SLayer storage configuration across implementation and documentation layers. The core change introduces explicit platform detection for default storage paths in ChangesPlatform-aware default storage paths
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/dbt/dbt_import.md (1)
10-10:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove legacy hardcoded storage path from Quick Start.
Quick Start still hardcodes
--storage ./slayer_data, which conflicts with the new platform-default guidance and can mislead users into thinking the legacy path is still the default.Suggested doc fix
- slayer import-dbt ./my_dbt_project --datasource my_postgres --storage ./slayer_data + slayer import-dbt ./my_dbt_project --datasource my_postgres🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/dbt/dbt_import.md` at line 10, The Quick Start example includes a legacy hardcoded storage flag (--storage ./slayer_data) in the "slayer import-dbt ./my_dbt_project --datasource my_postgres --storage ./slayer_data" command; remove the --storage ./slayer_data part from the example (or replace it with an optional note such as "[--storage <path>] to override platform default") so the example shows the platform-default behavior and avoids implying the old path is the default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/dbt/dbt_import.md`:
- Line 10: The Quick Start example includes a legacy hardcoded storage flag
(--storage ./slayer_data) in the "slayer import-dbt ./my_dbt_project
--datasource my_postgres --storage ./slayer_data" command; remove the --storage
./slayer_data part from the example (or replace it with an optional note such as
"[--storage <path>] to override platform default") so the example shows the
platform-default behavior and avoids implying the old path is the default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 84b93603-b5b2-4d1d-9ec0-be9329a213b7
📒 Files selected for processing (10)
CLAUDE.mddocs/configuration/storage.mddocs/dbt/dbt_import.mddocs/getting-started/mcp.mddocs/interfaces/cli.mddocs/interfaces/mcp.mddocs/interfaces/rest-api.mddocs/reference/cli.mddocs/reference/mcp.mdslayer/storage/base.py
| ## CLI | ||
|
|
||
| - All commands accept `--storage` (directory for YAML, `.db` file for SQLite). Defaults to platform-appropriate path (`~/.local/share/slayer` on Linux, `~/Library/Application Support/slayer` on macOS). Override with `$SLAYER_STORAGE` env var. Legacy `--models-dir` still works. | ||
| - All commands accept `--storage` (directory for YAML, `.db` file for SQLite). Defaults to platform-appropriate path (`~/.local/share/slayer` on Linux, `~/Library/Application Support/slayer` on macOS, `%LOCALAPPDATA%\slayer` on Windows). Override with `$SLAYER_STORAGE` env var. |
There was a problem hiding this comment.
It should be ~/.local/share/slayer on Mac as well
| |---|---| | ||
| | Linux | `~/.local/share/slayer` (or `$XDG_DATA_HOME/slayer`) | | ||
| | Linux | `~/.local/share/slayer` (or `$XDG_DATA_HOME/slayer` if set) | | ||
| | macOS | `~/Library/Application Support/slayer` | |



Summary by CodeRabbit
./slayer_data--storageflag and$SLAYER_STORAGEenvironment variable