Update sql-to-falkordb.md to reflect supabase support, and deployment…#471
Open
shahar-biron wants to merge 3 commits into
Open
Update sql-to-falkordb.md to reflect supabase support, and deployment…#471shahar-biron wants to merge 3 commits into
shahar-biron wants to merge 3 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR updates the SQL-to-FalkorDB migration guide with concrete sample config file paths across multiple SQL loaders (Databricks, PostgreSQL, Snowflake, Spark SQL), documents optional purge modes and schema scaffolding with specific example configs, clarifies control-plane run execution semantics (local vs Kubernetes) with new environment variables, adds a new container+Kubernetes single-deployment model section with build and Helm commands, and adds three new terms to the global wordlist. ChangesSQL-to-FalkorDB Migration Documentation
Wordlist Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
operations/migration/sql-to-falkordb.md (1)
160-160: ⚡ Quick winConsider expanding Supabase guidance beyond SSL mode.
While the SSL mode note is technically correct, the PR title emphasizes "reflect supabase support," but this single line is the only Supabase-specific guidance. Consider enhancing this section with:
- An example
POSTGRES_URLconnection string for Supabase (showing the SSL mode in context)- A note about Supabase's connection pooler vs. direct connection options
- Optional: A dedicated Supabase subsection or callout box for visibility
This would better align the documentation with the PR's stated objective and provide more practical guidance for Supabase users.
📝 Example enhancement
Note: PostgreSQL currently supports daemon mode but not purge flags. -For Supabase-compatible Postgres endpoints, include `sslmode=require` (minimum) or `sslmode=verify-full` in `POSTGRES_URL`. + +#### Supabase + +Supabase-compatible Postgres endpoints require SSL. Include `sslmode=require` (minimum) or `sslmode=verify-full` in your `POSTGRES_URL`: + +```bash +# Example Supabase connection string +export POSTGRES_URL="postgresql://postgres:[YOUR-PASSWORD]@db.[YOUR-PROJECT-REF].supabase.co:5432/postgres?sslmode=require" +``` + +Note: Supabase provides both a direct connection string and a connection pooler URL (port 6543). Either can be used depending on your concurrency requirements.🤖 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 `@operations/migration/sql-to-falkordb.md` at line 160, The Supabase guidance is too minimal—update the section around POSTGRES_URL to include a concrete example Supabase connection string (showing sslmode=require or sslmode=verify-full in context), add a short note explaining Supabase provides both a direct DB URL and a pooled URL (connection pooler on port 6543) and when to prefer each, and promote this into a small "Supabase" subsection or callout so users can easily find the POSTGRES_URL example and pooler guidance.
🤖 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.
Inline comments:
In `@operations/migration/sql-to-falkordb.md`:
- Around line 222-225: The scaffold examples use absolute repo-root paths
(PostgreSQL-to-FalkorDB/postgres-to-falkordb/example.config.yaml and
PostgreSQL-to-FalkorDB/postgres.generated.template.yaml) but the PostgreSQL
section earlier instructs readers to cd into
PostgreSQL-to-FalkorDB/postgres-to-falkordb; update these two example cargo
commands to use local paths (example.config.yaml and
./postgres.generated.template.yaml) or add a preceding note that the working
directory is the postgres-to-falkordb folder so the current paths are correct,
referencing the cargo run invocations and the
example.config.yaml/postgres.generated.template.yaml filenames to locate the
lines to change.
- Around line 210-213: The purge example commands use paths like
ClickHouse-to-FalkorDB/clickhouse.incremental.yaml which conflict with earlier
instructions that change the working directory into ClickHouse-to-FalkorDB;
update the two example lines (the cargo run commands that include --config
ClickHouse-to-FalkorDB/clickhouse.incremental.yaml and the --purge-mapping
example) so their config path is relative to the current working directory
(e.g., use --config clickhouse.incremental.yaml), or alternatively prefix the
section with a note stating these commands must be executed from the repository
root; make the change in the SQL-to-FalkorDB migration docs where the purge
examples appear to ensure consistency with the earlier "cd
ClickHouse-to-FalkorDB" instruction.
---
Nitpick comments:
In `@operations/migration/sql-to-falkordb.md`:
- Line 160: The Supabase guidance is too minimal—update the section around
POSTGRES_URL to include a concrete example Supabase connection string (showing
sslmode=require or sslmode=verify-full in context), add a short note explaining
Supabase provides both a direct DB URL and a pooled URL (connection pooler on
port 6543) and when to prefer each, and promote this into a small "Supabase"
subsection or callout so users can easily find the POSTGRES_URL example and
pooler guidance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c8f77b7f-d36e-4924-b3b3-8552d642db3d
📒 Files selected for processing (1)
operations/migration/sql-to-falkordb.md
Comment on lines
+210
to
+213
| cargo run --release -- --config ClickHouse-to-FalkorDB/clickhouse.incremental.yaml --purge-graph | ||
|
|
||
| # Purge selected mappings | ||
| cargo run --release -- --config path/to/config.yaml --purge-mapping customers | ||
| cargo run --release -- --config ClickHouse-to-FalkorDB/clickhouse.incremental.yaml --purge-mapping customers |
Contributor
There was a problem hiding this comment.
Fix path inconsistency in purge examples.
The paths ClickHouse-to-FalkorDB/clickhouse.incremental.yaml assume running from the repository root, but the ClickHouse section (line 92) instructs users to cd ClickHouse-to-FalkorDB first. If users follow that instruction and remain in that directory, these purge commands will fail with "file not found" errors.
Either:
- Clarify that purge examples should be run from the repository root, OR
- Adjust the paths to match the working directory established in Option A sections
🔧 Proposed fix (Option 2: adjust paths)
# Purge full graph before loading
-cargo run --release -- --config ClickHouse-to-FalkorDB/clickhouse.incremental.yaml --purge-graph
+cargo run --release -- --config clickhouse.incremental.yaml --purge-graph
# Purge selected mappings
-cargo run --release -- --config ClickHouse-to-FalkorDB/clickhouse.incremental.yaml --purge-mapping customers
+cargo run --release -- --config clickhouse.incremental.yaml --purge-mapping customersOr add a note:
+Note: The following examples assume you are in the repository root directory.
+
# Purge full graph before loading
cargo run --release -- --config ClickHouse-to-FalkorDB/clickhouse.incremental.yaml --purge-graph📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cargo run --release -- --config ClickHouse-to-FalkorDB/clickhouse.incremental.yaml --purge-graph | |
| # Purge selected mappings | |
| cargo run --release -- --config path/to/config.yaml --purge-mapping customers | |
| cargo run --release -- --config ClickHouse-to-FalkorDB/clickhouse.incremental.yaml --purge-mapping customers | |
| cargo run --release -- --config clickhouse.incremental.yaml --purge-graph | |
| # Purge selected mappings | |
| cargo run --release -- --config clickhouse.incremental.yaml --purge-mapping customers |
🤖 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 `@operations/migration/sql-to-falkordb.md` around lines 210 - 213, The purge
example commands use paths like
ClickHouse-to-FalkorDB/clickhouse.incremental.yaml which conflict with earlier
instructions that change the working directory into ClickHouse-to-FalkorDB;
update the two example lines (the cargo run commands that include --config
ClickHouse-to-FalkorDB/clickhouse.incremental.yaml and the --purge-mapping
example) so their config path is relative to the current working directory
(e.g., use --config clickhouse.incremental.yaml), or alternatively prefix the
section with a note stating these commands must be executed from the repository
root; make the change in the SQL-to-FalkorDB migration docs where the purge
examples appear to ensure consistency with the earlier "cd
ClickHouse-to-FalkorDB" instruction.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.wordlist.txt (1)
792-794: ⚡ Quick winConsider maintaining alphabetical order for better maintainability.
The new entries are appended to the end, but the wordlist has alphabetically organized sections. For improved searchability and consistency, consider placing them in alphabetical order:
- "kubectl" → after "kbcli" (line 659)
- "Postgres" → in the 'P' section (around lines 253-272)
- "Supabase" → in the 'S' section (around lines 531-575)
This refactor can be deferred if the team prefers appending new entries.
🤖 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 @.wordlist.txt around lines 792 - 794, The new wordlist entries "kubectl", "Postgres", and "Supabase" are appended at the end and should be moved into their alphabetized sections: insert "kubectl" immediately after the existing "kbcli" entry, place "Postgres" within the 'P' section near other P-words, and place "Supabase" within the 'S' section alongside S-entries; ensure each entry follows the file's casing/formatting rules and update any surrounding separators or indexes so alphabetical ordering is preserved.
🤖 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.
Nitpick comments:
In @.wordlist.txt:
- Around line 792-794: The new wordlist entries "kubectl", "Postgres", and
"Supabase" are appended at the end and should be moved into their alphabetized
sections: insert "kubectl" immediately after the existing "kbcli" entry, place
"Postgres" within the 'P' section near other P-words, and place "Supabase"
within the 'S' section alongside S-entries; ensure each entry follows the file's
casing/formatting rules and update any surrounding separators or indexes so
alphabetical ordering is preserved.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the SQL-to-FalkorDB online migration guide to reflect Supabase/Postgres connection requirements and to document control-plane execution via either local or Kubernetes backends, including a consolidated container + Helm deployment model.
Changes:
- Replaced placeholder loader commands with concrete example config filenames/paths across multiple source loaders.
- Documented control-plane execution backend selection (
localvskubernetes) and added related environment variables. - Added a “single-deployment model” section describing building images and installing via Helm; updated spellcheck wordlist for new terms.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| operations/migration/sql-to-falkordb.md | Updates loader examples, adds Supabase SSL note, expands control-plane config with Kubernetes execution options, and documents container+Helm deployment model. |
| .wordlist.txt | Adds new glossary/wordlist terms used by the updated documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
208
to
+213
| ```bash | ||
| # Purge full graph before loading | ||
| cargo run --release -- --config path/to/config.yaml --purge-graph | ||
| cargo run --release -- --config ClickHouse-to-FalkorDB/clickhouse.incremental.yaml --purge-graph | ||
|
|
||
| # Purge selected mappings | ||
| cargo run --release -- --config path/to/config.yaml --purge-mapping customers | ||
| cargo run --release -- --config ClickHouse-to-FalkorDB/clickhouse.incremental.yaml --purge-mapping customers |
Comment on lines
221
to
+225
| # Print normalized source schema summary | ||
| cargo run --release -- --config path/to/config.yaml --introspect-schema | ||
| cargo run --release -- --config PostgreSQL-to-FalkorDB/postgres-to-falkordb/example.config.yaml --introspect-schema | ||
|
|
||
| # Generate starter mapping template | ||
| cargo run --release -- --config path/to/config.yaml --generate-template --output scaffold.yaml | ||
| cargo run --release -- --config PostgreSQL-to-FalkorDB/postgres-to-falkordb/example.config.yaml --generate-template --output PostgreSQL-to-FalkorDB/postgres.generated.template.yaml |
Comment on lines
+269
to
+276
| - `CONTROL_PLANE_K8S_NAMESPACE` (namespace where Kubernetes run workloads are created) | ||
| - `CONTROL_PLANE_K8S_RUNNER_IMAGE` (multi-tool runner image reference) | ||
| - `CONTROL_PLANE_K8S_IMAGE_PULL_POLICY` (runner image pull policy) | ||
| - `CONTROL_PLANE_K8S_SERVICE_ACCOUNT` (service account used for run workloads) | ||
| - `CONTROL_PLANE_K8S_SHARED_PVC` (optional shared PVC for file-backed state) | ||
| - `CONTROL_PLANE_K8S_ENV_SECRET` / `CONTROL_PLANE_K8S_ENV_CONFIGMAP` (optional env sources projected to run pods) | ||
| - `CONTROL_PLANE_K8S_KUBECTL_BIN` (kubectl binary path; default `kubectl`) | ||
| - `CONTROL_PLANE_K8S_BINARY_DIR` (tool binary directory in runner image; default `/opt/falkordb/bin`) |
Comment on lines
+308
to
+316
| ## Container + Kubernetes single-deployment model | ||
|
|
||
| The DM-SQL repository includes a single-release deployment path that pairs one control-plane instance with one multi-tool runner image: | ||
|
|
||
| - `control-plane/Dockerfile` builds the control-plane API + UI image. | ||
| - `docker/runner.Dockerfile` builds a runner image containing all SQL-to-FalkorDB binaries. | ||
| - `docker/build-images.sh <version> [registry]` builds both images with one version tag. | ||
| - `deploy/helm/dm-sql-to-falkordb/` provides a Helm chart for a unified deployment. | ||
|
|
… of the control plane over k8s.
… dictionary Co-Authored-By: Oz <oz-agent@warp.dev>
shahar-biron
force-pushed
the
sql-to-falkordb-supabase-k8s
branch
from
116e261 to
5026c71
Compare
Contributor
|
@copilot resolve the merge conflicts in this pull request |
Contributor
Merge conflicts are resolved in commit |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
operations/migration/sql-to-falkordb.md (2)
210-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPath inconsistency in purge examples—still unresolved.
Lines 210 and 213 use the full repo-root path
ClickHouse-to-FalkorDB/clickhouse.incremental.yaml, but the ClickHouse one-shot examples at lines 96–99 use the directory-relative pathclickhouse.incremental.yamlaftercd ClickHouse-to-FalkorDB(line 92). Users following the established pattern will encounter "file not found" errors when running these purge commands.This issue was previously flagged and marked as addressed, but the inconsistency persists in the current code.
Recommended fix: Align with the working directory established in the ClickHouse section (line 92) by using
clickhouse.incremental.yamlinstead of the full path.🔧 Proposed fix
# Purge full graph before loading -cargo run --release -- --config ClickHouse-to-FalkorDB/clickhouse.incremental.yaml --purge-graph +cargo run --release -- --config clickhouse.incremental.yaml --purge-graph # Purge selected mappings -cargo run --release -- --config ClickHouse-to-FalkorDB/clickhouse.incremental.yaml --purge-mapping customers +cargo run --release -- --config clickhouse.incremental.yaml --purge-mapping customers🤖 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 `@operations/migration/sql-to-falkordb.md` around lines 210 - 213, The purge command examples use the full path ClickHouse-to-FalkorDB/clickhouse.incremental.yaml while earlier ClickHouse examples establish a working directory of ClickHouse-to-FalkorDB and use the relative path clickhouse.incremental.yaml. Update both purge command examples (the one with --purge-graph flag and the one with --purge-mapping customers flag) to use the relative path clickhouse.incremental.yaml instead of the full path prefix to maintain consistency with the established working directory pattern and ensure users do not encounter file-not-found errors when running these commands.
222-225:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPath inconsistency in scaffold examples—unresolved.
Lines 222 and 225 use the full repo-root paths
PostgreSQL-to-FalkorDB/postgres-to-falkordb/example.config.yamlandPostgreSQL-to-FalkorDB/postgres.generated.template.yaml, but the PostgreSQL one-shot examples at lines 153–156 use the directory-relative pathexample.config.yamlaftercd PostgreSQL-to-FalkorDB/postgres-to-falkordb(line 149). Users following the established pattern will encounter "file not found" errors.This issue was previously flagged and marked as addressed, but it persists in the current code.
Recommended fix: Align with the working directory established in the PostgreSQL section (line 149) by using relative paths.
🔧 Proposed fix
# Print normalized source schema summary -cargo run --release -- --config PostgreSQL-to-FalkorDB/postgres-to-falkordb/example.config.yaml --introspect-schema +cargo run --release -- --config example.config.yaml --introspect-schema # Generate starter mapping template -cargo run --release -- --config PostgreSQL-to-FalkorDB/postgres-to-falkordb/example.config.yaml --generate-template --output PostgreSQL-to-FalkorDB/postgres.generated.template.yaml +cargo run --release -- --config example.config.yaml --generate-template --output ../postgres.generated.template.yaml🤖 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 `@operations/migration/sql-to-falkordb.md` around lines 222 - 225, The cargo commands on lines 222 and 225 use full repository paths (PostgreSQL-to-FalkorDB/postgres-to-falkordb/example.config.yaml and PostgreSQL-to-FalkorDB/postgres.generated.template.yaml) which are inconsistent with the working directory established by the cd command at line 149 (cd PostgreSQL-to-FalkorDB/postgres-to-falkordb). Update the paths on lines 222 and 225 to use relative paths instead: change the config path reference to just example.config.yaml, and change the output path to use a relative path like ../postgres.generated.template.yaml to correctly reference the parent directory, matching the pattern established in the earlier PostgreSQL one-shot examples.
🧹 Nitpick comments (1)
operations/migration/sql-to-falkordb.md (1)
312-314: 💤 Low valueMinor: Reduce sentence-beginning repetition.
The three list items (lines 312–314) all begin with a backtick-enclosed filename followed by the word "builds," creating parallel structure that the LanguageTool style checker flags. Consider varying the phrasing for better readability.
✏️ Suggested reworking
- `control-plane/Dockerfile` builds the control-plane API + UI image. -- `docker/runner.Dockerfile` builds a runner image containing all SQL-to-FalkorDB binaries. -- `docker/build-images.sh <version> [registry]` builds both images with one version tag. +- `docker/runner.Dockerfile` creates a runner image containing all SQL-to-FalkorDB binaries. +- `docker/build-images.sh <version> [registry]` packages both images with one version tag.🤖 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 `@operations/migration/sql-to-falkordb.md` around lines 312 - 314, The three list items in the markdown file starting at line 312 all follow the same repetitive pattern of "backtick-enclosed filename followed by builds," which reduces readability. Rephrase each of the three bullet points to vary the sentence structure and word choice while maintaining the same informational content about what each file or script does. Consider restructuring some items to begin with different grammatical constructions rather than the filename-builds pattern.
🤖 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.
Duplicate comments:
In `@operations/migration/sql-to-falkordb.md`:
- Around line 210-213: The purge command examples use the full path
ClickHouse-to-FalkorDB/clickhouse.incremental.yaml while earlier ClickHouse
examples establish a working directory of ClickHouse-to-FalkorDB and use the
relative path clickhouse.incremental.yaml. Update both purge command examples
(the one with --purge-graph flag and the one with --purge-mapping customers
flag) to use the relative path clickhouse.incremental.yaml instead of the full
path prefix to maintain consistency with the established working directory
pattern and ensure users do not encounter file-not-found errors when running
these commands.
- Around line 222-225: The cargo commands on lines 222 and 225 use full
repository paths
(PostgreSQL-to-FalkorDB/postgres-to-falkordb/example.config.yaml and
PostgreSQL-to-FalkorDB/postgres.generated.template.yaml) which are inconsistent
with the working directory established by the cd command at line 149 (cd
PostgreSQL-to-FalkorDB/postgres-to-falkordb). Update the paths on lines 222 and
225 to use relative paths instead: change the config path reference to just
example.config.yaml, and change the output path to use a relative path like
../postgres.generated.template.yaml to correctly reference the parent directory,
matching the pattern established in the earlier PostgreSQL one-shot examples.
---
Nitpick comments:
In `@operations/migration/sql-to-falkordb.md`:
- Around line 312-314: The three list items in the markdown file starting at
line 312 all follow the same repetitive pattern of "backtick-enclosed filename
followed by builds," which reduces readability. Rephrase each of the three
bullet points to vary the sentence structure and word choice while maintaining
the same informational content about what each file or script does. Consider
restructuring some items to begin with different grammatical constructions
rather than the filename-builds pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eec71b42-6fdb-4641-b676-e2755adb3592
📒 Files selected for processing (2)
.wordlist.txtoperations/migration/sql-to-falkordb.md
✅ Files skipped from review due to trivial changes (1)
- .wordlist.txt
galshubeli
approved these changes
Jun 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
… of the control plane over k8s.
Summary by CodeRabbit
kubectl,Postgres, andSupabase.