Improve fly docs add badge #186
Conversation
- Added Fly.io deployment button to README for easier access. - Updated Fly.io deployment documentation to reflect the switch from PostgreSQL to SQLite storage. - Revised configuration settings in `dagster_fly.yaml` for improved performance and resource allocation. - Enhanced instructions for accessing and managing the SQLite database.
|
Caution Review failedThe pull request is closed. ## Walkthrough
A Fly.io deployment badge was added to the README, linking to deployment documentation. The Fly.io deployment guide was extensively revised to replace PostgreSQL with SQLite on persistent volumes, updating configuration, architecture, troubleshooting, and cost sections to reflect this change. A test was updated to clarify environment variable precedence over YAML configuration for a specific parameter. No code or public API changes were made.
## Changes
| File(s) | Change Summary |
|-----------------------------|-------------------------------------------------------------------------------------------------------|
| README.md | Added Fly.io deployment badge linking to documentation. |
| docs/docs/deployment/fly.md | Revised deployment guide: replaced PostgreSQL with SQLite on persistent volumes; updated configs, architecture, troubleshooting, and cost estimates accordingly. |
| tests/test_config.py | Modified test to reflect conditional precedence of environment variables over YAML for `table_key` parameter. |
## Estimated code review effort
3 (~40 minutes)
## Poem
> A badge now shines with Fly.io’s hue,
> And docs take flight with something new.
> Goodbye, Postgres—SQLite’s in view,
> On volumes persistent, robust and true.
> Deployments are lighter, the guide is refreshed—
> The stack hops forward, thoroughly blessed!
> 🐇✨📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Pull Request Overview
This pull request updates the Anomstack deployment documentation to transition from PostgreSQL to SQLite for data storage on Fly.io deployments. The changes simplify the storage architecture while also adding a deployment badge for better accessibility.
- Migrates from managed PostgreSQL to SQLite storage on persistent volumes
- Updates VM configuration with increased resources (8GB RAM, 4 CPUs) for ML workloads
- Adds Fly.io deployment badge to README for easier access
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/docs/deployment/fly.md | Complete migration from PostgreSQL to SQLite storage, updated configuration examples, cost estimates, and troubleshooting guides |
| README.md | Added Fly.io deployment badge linking to documentation |
| ``` | ||
|
|
There was a problem hiding this comment.
The VM configuration shows 8GB RAM but the cost estimation table still references smaller memory configurations (1GB, 2GB, 4GB) that don't align with this default configuration. Consider updating the cost table to reflect realistic configurations or explaining the discrepancy.
| ``` | |
| ## Cost Estimation Table | |
| The following table provides an estimate of costs for different VM configurations. The default configuration uses 8GB RAM, but you can adjust this based on your workload requirements. | |
| | Memory (RAM) | CPUs | Estimated Cost (per month) | | |
| |--------------|------|----------------------------| | |
| | 1GB | 1 | $5 | | |
| | 2GB | 2 | $10 | | |
| | 4GB | 2 | $20 | | |
| | 8GB (default)| 4 | $40 | | |
| Note: Costs are approximate and may vary based on your region and usage. |
| ``` | ||
|
|
There was a problem hiding this comment.
The VM configuration specifies 'performance' CPU kind with 4 CPUs, but the cost estimation table references 'shared-cpu-1x', 'shared-cpu-2x', and 'dedicated-cpu-1x' configurations. This inconsistency between the example configuration and cost estimates may confuse users about actual deployment costs.
| ``` | |
| # Cost Estimation | |
| # The following cost estimates are based on the VM configuration specified above: | |
| # - **Performance CPU (4 CPUs)**: Estimated cost is $XX/month (replace with actual cost) | |
| # - **Memory (8GB)**: Included in the above estimate | |
| # Note: Adjust the configuration and recalculate costs as needed for your use case. |
- Updated test to clarify that environment variables override YAML values except for table_key, which respects YAML precedence. - Enhanced assertions to ensure table_key is validated correctly based on its definition in YAML.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/docs/deployment/fly.md (1)
468-472: Cost table still lists 1–4 GB tiers – inconsistent with default 8 GB VMSame feedback was given previously; please align the examples or add a note explaining the discrepancy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)docs/docs/deployment/fly.md(10 hunks)
🔇 Additional comments (4)
README.md (1)
7-8: Check badge target – link appears mis-aligned with new docs pathThe badge points at
https://andrewm4894.github.io/anomstack/docs/deployment/fly
but the new guide lives underdocs/docs/deployment/fly.md. Unless the site generator flattens the folder structure, this URL will 404.Confirm the rendered URL in GitHub-Pages and update if required:
-[](https://andrewm4894.github.io/anomstack/docs/deployment/fly) +[](https://andrewm4894.github.io/anomstack/docs/docs/deployment/fly)docs/docs/deployment/fly.md (3)
264-266: Verify Dockerfile path in[build]section
docker/Dockerfile.flymust exist at runtime orfly deploywill fail with “Build failed: stat …”.
Double-check the exact relative path (most repos keep it at project root).If the file is actually
Dockerfile.flyat repo root:-[build] - dockerfile = "docker/Dockerfile.fly" +[build] + dockerfile = "Dockerfile.fly"
518-526: Example “database connection test” still imports DuckDBThe doc now standardises on SQLite, yet the troubleshooting snippet below continues to test DuckDB connectivity. Replace with a simple
sqlite3check (you already used one above) to prevent misleading readers.-import duckdb -conn = duckdb.connect('/data/anomstack.db') -print('DuckDB connection successful!') +import sqlite3 +conn = sqlite3.connect('/data/anomstack.db') +print('SQLite connection successful!')Likely an incorrect or invalid review comment.
420-431: Section still mixes SQLite & DuckDB terminologyThe header correctly says “SQLite”, but commands/captions reference DuckDB and copy the database to
/data/anomstack.db, which is SQLite. Rename the comment and clarify tooling to avoid user confusion:-# Access SQLite database via SSH +# Access SQLite database via SSH -# Create backup of SQLite database +# Create backup of SQLite database -# View database size and usage +# View database size and usageOptionally show
sqlite3CLI usage rather thancponly.Likely an incorrect or invalid review comment.
| [vm] | ||
| memory = "2048" | ||
| cpu_kind = "shared" | ||
| cpus = 1 | ||
| memory = "8192" # 8GB RAM for Dagster ML workloads | ||
| cpu_kind = "performance" | ||
| cpus = 4 |
There was a problem hiding this comment.
memory should be an integer, not a string – Fly TOML will reject quotes
flyctl expects:
[vm]
memory = 8192 # integer ‑- no quotesCurrent quoted string will raise:
Error: invalid type: string "8192", expected u64
Apply:
- memory = "8192" # 8GB RAM for Dagster ML workloads
+ memory = 8192 # 8 GB RAM for Dagster ML workloads🤖 Prompt for AI Agents
In docs/docs/deployment/fly.md at lines 287 to 290, the memory value in the [vm]
section is incorrectly set as a string with quotes. Remove the quotes around
8192 so that memory is assigned as an integer without quotes, like memory =
8192, to comply with Fly TOML requirements and avoid type errors.
📊 Test Coverage ReportCoverage: 59% (yellow) ✅ Coverage maintained or improved!
|
This pull request updates the deployment documentation for Anomstack on Fly.io, transitioning from PostgreSQL to SQLite for storage. It also introduces changes to the deployment configuration and cost estimates, reflecting this new storage approach. Below is a summary of the most important changes:
Transition from PostgreSQL to SQLite:
docs/docs/deployment/fly.md). [1] [2] [3]docs/docs/deployment/fly.md). [1] [2]Deployment Configuration Updates:
dockerfilereference in the Fly.io configuration and updated VM settings to allocate more resources (8GB RAM, 4 CPUs) for machine learning workloads (docs/docs/deployment/fly.md). [1] [2]Cost Estimation Adjustments:
docs/docs/deployment/fly.md).Documentation Enhancements:
README.mdfor easier access to deployment instructions (README.md).docs/docs/deployment/fly.md).Summary by CodeRabbit