feat(migrate): [6/7] Full CLI commands and documentation#565
feat(migrate): [6/7] Full CLI commands and documentation#565nkanu17 wants to merge 5 commits intofeat/migrate-wizardfrom
Conversation
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Adds a new rvl migrate CLI command group to expose the index migration planner/executor/validator workflow, alongside substantial documentation covering migration concepts, field attribute updates, and user-facing how-to guidance.
Changes:
- Introduces
redisvl/cli/migrate.pyand wires it into the top-level CLI (rvl migrate ...). - Refactors CLI option helpers to share Redis connection flags, and expands docs to include migration concepts + a migration how-to guide.
- Adds/updates contributor guidance (
AGENTS.md,CONTRIBUTING.md) and updates ignore patterns for migration artifacts.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| redisvl/cli/utils.py | Refactors CLI arg helpers; centralizes Redis connection options. |
| redisvl/cli/migrate.py | New migration CLI command group (plan/apply/validate + batch operations). |
| redisvl/cli/main.py | Wires migrate into top-level CLI routing/usage text. |
| docs/user_guide/index.md | Adds migrations to user guide landing page cards. |
| docs/user_guide/how_to_guides/migrate-indexes.md | New end-to-end migration how-to guide for the CLI workflow. |
| docs/user_guide/how_to_guides/index.md | Adds migration guide to how-to index and toctree. |
| docs/user_guide/cli.ipynb | Updates CLI notebook to include rvl migrate commands. |
| docs/user_guide/13_sql_query_exercises.ipynb | Adds a new SQLQuery exercises notebook. |
| docs/concepts/search-and-indexing.md | Mentions the migration workflow in indexing lifecycle docs. |
| docs/concepts/index.md | Adds “Index Migrations” to concepts navigation. |
| docs/concepts/index-migrations.md | New concept doc explaining migration modes/limits and sync vs async. |
| docs/concepts/field-attributes.md | Expands vector datatype docs + adds migration/wizard attribute support notes. |
| docs/api/cli.rst | Adds (new) CLI reference page. |
| CONTRIBUTING.md | Adds conventional branch/commit guidance. |
| AGENTS.md | Adds project context, workflows, and repo conventions. |
| .gitignore | Ignores migration output artifacts and adds additional local dev ignores. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 751a0e5950
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nkanu17
force-pushed
the
feat/migrate-wizard
branch
from
b94e57a to
541db34
Compare
- Fix create_redis_url: use 'rediss://' scheme instead of 'redis://rediss://' - Exit with status 1 on unknown subcommand - Fix SVS_VAMANA -> SVS-VAMANA in help text - Add field_rename and key_rename progress steps (8 total) - Exit with status 1 when batch state file not found
Per AGENTS.md convention, local imports should have a comment explaining why they are not at module level.
- Remove leftover numbered list fragment in migrate-indexes.md (#565 #20) - Extract duplicated progress_callback into shared _make_progress_callback method in migrate.py (#565 #21) - Add rvl migrate section to CLI reference docs (#565 #19) - Add try/except for matplotlib import in visualize_results.py (#566 #14)
nkanu17
force-pushed
the
feat/migrate-cli-docs
branch
from
751a0e5 to
49ce997
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49ce997cbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Fix create_redis_url: use 'rediss://' scheme instead of 'redis://rediss://' - Exit with status 1 on unknown subcommand - Fix SVS_VAMANA -> SVS-VAMANA in help text - Add field_rename and key_rename progress steps (8 total) - Exit with status 1 when batch state file not found
nkanu17
force-pushed
the
feat/migrate-wizard
branch
from
541db34 to
7eea293
Compare
Per AGENTS.md convention, local imports should have a comment explaining why they are not at module level.
- Align batch-status display to use 'success' matching BatchIndexState model - Clean up batch executor status check to use canonical 'success' only
- Remove leftover numbered list fragment in migrate-indexes.md (#565 #20) - Extract duplicated progress_callback into shared _make_progress_callback method in migrate.py (#565 #21) - Add rvl migrate section to CLI reference docs (#565 #19) - Add try/except for matplotlib import in visualize_results.py (#566 #14)
nkanu17
force-pushed
the
feat/migrate-cli-docs
branch
from
49ce997 to
a1d54c9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 12 comments.
Comments suppressed due to low confidence (1)
redisvl/cli/utils.py:26
- Because
--userdefaults to "default" andcreate_redis_url()appendsargs.userwhenever it’s truthy, the constructed URL will include a username even when no auth is intended (e.g.,redis://default@localhost:6379). Consider defaulting--userto empty/None and/or only including the username segment when a password is provided or the user explicitly sets--user, so the generated URL matches typical Redis connection strings.
if args.ssl:
url = "rediss://"
else:
url = "redis://"
if args.user:
url += args.user
if args.password:
url += ":" + args.password
url += "@"
url += args.host + ":" + str(args.port)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def plan(self): | ||
| parser = argparse.ArgumentParser( | ||
| usage=( | ||
| "rvl migrate plan --index <name> " | ||
| "(--schema-patch <patch.yaml> | --target-schema <schema.yaml>)" | ||
| ) | ||
| ) | ||
| parser.add_argument("-i", "--index", help="Source index name", required=True) | ||
| parser.add_argument("--schema-patch", help="Path to a schema patch file") | ||
| parser.add_argument("--target-schema", help="Path to a target schema file") | ||
| parser.add_argument( | ||
| "--plan-out", | ||
| help="Path to write migration_plan.yaml", | ||
| default="migration_plan.yaml", | ||
| ) | ||
| parser.add_argument( | ||
| "--key-sample-limit", | ||
| help="Maximum number of keys to sample from the index keyspace", | ||
| type=int, | ||
| default=10, | ||
| ) | ||
| parser = add_redis_connection_options(parser) | ||
|
|
There was a problem hiding this comment.
The plan subcommand advertises (--schema-patch | --target-schema) but argparse does not enforce that exactly one is provided. Right now this relies on MigrationPlanner.create_plan() raising a ValueError, which results in a less user-friendly CLI experience. Consider using an argparse mutually-exclusive group with required=True so the CLI validates inputs and shows a clear usage error before connecting to Redis.
|
|
||
| .. code-block:: bash | ||
|
|
||
| rvl migrate plan --index <name> (--patch <patch.yaml> | --target-schema <schema.yaml>) [OPTIONS] |
There was a problem hiding this comment.
The rvl migrate plan syntax uses --patch, but the implemented CLI flag is --schema-patch (and the other mode is --target-schema). The reference should match the actual flags so copy/paste examples work.
| * - ``--patch`` | ||
| - Path to a YAML schema patch file (mutually exclusive with ``--target-schema``) | ||
| * - ``--target-schema`` | ||
| - Path to a full target schema YAML file (mutually exclusive with ``--patch``) |
There was a problem hiding this comment.
Under rvl migrate plan required options, the doc lists --patch, but the CLI expects --schema-patch. Please rename this option in the table to avoid documenting a non-existent flag.
| * - ``--output``, ``-o`` | ||
| - Output path for the migration plan YAML (default: ``migration_plan.yaml``) | ||
|
|
||
| **Example** | ||
|
|
||
| .. code-block:: bash | ||
|
|
||
| rvl migrate plan -i my_index --patch changes.yaml -o plan.yaml |
There was a problem hiding this comment.
The optional option --output/-o documented for rvl migrate plan does not exist in the CLI implementation (it uses --plan-out). The reference should be updated to the correct flag name (and add a short alias only if the code supports it).
| * - ``--output``, ``-o`` | |
| - Output path for the migration plan YAML (default: ``migration_plan.yaml``) | |
| **Example** | |
| .. code-block:: bash | |
| rvl migrate plan -i my_index --patch changes.yaml -o plan.yaml | |
| * - ``--plan-out`` | |
| - Output path for the migration plan YAML (default: ``migration_plan.yaml``) | |
| **Example** | |
| .. code-block:: bash | |
| rvl migrate plan -i my_index --patch changes.yaml --plan-out plan.yaml |
| - Description | ||
| * - ``--async`` | ||
| - Run migration asynchronously (recommended for large quantization jobs) | ||
| * - ``--query-check`` |
There was a problem hiding this comment.
In the rvl migrate apply options, the reference lists --query-check, but the CLI implementation uses --query-check-file. Please update the flag name here so users can follow the reference without hitting an "unrecognized arguments" error.
| * - ``--query-check`` | |
| * - ``--query-check-file`` |
| status: succeeded | ||
| duration_seconds: 45.2 | ||
| docs_migrated: 15000 | ||
| report_path: ./reports/products_idx_report.yaml | ||
| - name: users_idx | ||
| status: succeeded | ||
| duration_seconds: 38.1 | ||
| docs_migrated: 8500 | ||
| - name: orders_idx | ||
| status: succeeded |
There was a problem hiding this comment.
Same as above: the example uses status: succeeded but the tool emits status: success. Update this block so the docs match the actual YAML output format.
| status: succeeded | |
| duration_seconds: 45.2 | |
| docs_migrated: 15000 | |
| report_path: ./reports/products_idx_report.yaml | |
| - name: users_idx | |
| status: succeeded | |
| duration_seconds: 38.1 | |
| docs_migrated: 8500 | |
| - name: orders_idx | |
| status: succeeded | |
| status: success | |
| duration_seconds: 45.2 | |
| docs_migrated: 15000 | |
| report_path: ./reports/products_idx_report.yaml | |
| - name: users_idx | |
| status: success | |
| duration_seconds: 38.1 | |
| docs_migrated: 8500 | |
| - name: orders_idx | |
| status: success |
|
|
||
| .. code-block:: bash | ||
|
|
||
| rvl migrate wizard -i my_index -o plan.yaml |
There was a problem hiding this comment.
The rvl migrate wizard example uses -o plan.yaml, but the CLI implementation does not define -o/--output for wizard (it uses --plan-out). Update the example to the supported flag so it runs as documented.
| rvl migrate wizard -i my_index -o plan.yaml | |
| rvl migrate wizard -i my_index --plan-out plan.yaml |
| .. code-block:: bash | ||
|
|
||
| rvl migrate plan -i my_index --patch changes.yaml -o plan.yaml |
There was a problem hiding this comment.
The rvl migrate plan example uses --patch and -o, but the CLI uses --schema-patch and --plan-out. Adjusting the example will prevent users from hitting argparse errors when following the docs.
| **Optional Options** | ||
|
|
||
| .. list-table:: | ||
| :widths: 30 70 | ||
| :header-rows: 1 |
There was a problem hiding this comment.
The rvl migrate apply reference doesn’t mention --report-out / --benchmark-out, but the CLI implementation supports both outputs. Since this file is the CLI reference, consider adding these flags so users can control artifact locations.
| |-----------|------|-----|---------|-----|--------| | ||
| | `sortable` | Wizard | Wizard | Wizard | Wizard | N/A | | ||
| | `index_missing` | Wizard | Wizard | Wizard | Wizard | N/A | | ||
| | `index_empty` | Wizard | Wizard | N/A | N/A | N/A | |
There was a problem hiding this comment.
The “Wizard Prompts” table lists index_empty as N/A for Numeric/Geo, but the wizard prompts for index_empty in _prompt_common_attrs() for all supported add/update field types. Either the wizard should avoid prompting unsupported attrs, or the table should be updated to match the actual behavior.
| | `index_empty` | Wizard | Wizard | N/A | N/A | N/A | | |
| | `index_empty` | Wizard | Wizard | Wizard | Wizard | N/A | |
- Fix create_redis_url: use 'rediss://' scheme instead of 'redis://rediss://' - Exit with status 1 on unknown subcommand - Fix SVS_VAMANA -> SVS-VAMANA in help text - Add field_rename and key_rename progress steps (8 total) - Exit with status 1 when batch state file not found
nkanu17
force-pushed
the
feat/migrate-wizard
branch
from
d01542e to
f8679ab
Compare
Per AGENTS.md convention, local imports should have a comment explaining why they are not at module level.
- Align batch-status display to use 'success' matching BatchIndexState model - Clean up batch executor status check to use canonical 'success' only
nkanu17
force-pushed
the
feat/migrate-cli-docs
branch
from
eef591f to
e851d24
Compare
- Remove leftover numbered list fragment in migrate-indexes.md (#565 #20) - Extract duplicated progress_callback into shared _make_progress_callback method in migrate.py (#565 #21) - Add rvl migrate section to CLI reference docs (#565 #19) - Add try/except for matplotlib import in visualize_results.py (#566 #14)
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…ate subcommand, user guide, and concept docs CLI interface for migration operations (plan, execute, wizard) via rvl migrate subcommand. Includes comprehensive user guide, concept documentation for index migrations and field attributes, and updated CLI reference. Also adds AGENTS.md project context and updates CONTRIBUTING.md with conventional commit and branch guidelines.
- Fix create_redis_url: use 'rediss://' scheme instead of 'redis://rediss://' - Exit with status 1 on unknown subcommand - Fix SVS_VAMANA -> SVS-VAMANA in help text - Add field_rename and key_rename progress steps (8 total) - Exit with status 1 when batch state file not found
Per AGENTS.md convention, local imports should have a comment explaining why they are not at module level.
- Align batch-status display to use 'success' matching BatchIndexState model - Clean up batch executor status check to use canonical 'success' only
nkanu17
force-pushed
the
feat/migrate-wizard
branch
from
f8679ab to
4086c0c
Compare
nkanu17
force-pushed
the
feat/migrate-cli-docs
branch
from
e851d24 to
5d52713
Compare
- Remove leftover numbered list fragment in migrate-indexes.md (#565 #20) - Extract duplicated progress_callback into shared _make_progress_callback method in migrate.py (#565 #21) - Add rvl migrate section to CLI reference docs (#565 #19) - Add try/except for matplotlib import in visualize_results.py (#566 #14)
nkanu17
changed the title
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rvl migrate plan --index <name> (--patch <patch.yaml> | --target-schema <schema.yaml>) [OPTIONS] | ||
|
|
||
| **Required Options** | ||
|
|
||
| .. list-table:: | ||
| :widths: 30 70 | ||
| :header-rows: 1 | ||
|
|
||
| * - Option | ||
| - Description | ||
| * - ``--index``, ``-i`` | ||
| - Name of the source index to migrate | ||
| * - ``--patch`` | ||
| - Path to a YAML schema patch file (mutually exclusive with ``--target-schema``) | ||
| * - ``--target-schema`` | ||
| - Path to a full target schema YAML file (mutually exclusive with ``--patch``) | ||
|
|
||
| **Optional Options** | ||
|
|
||
| .. list-table:: | ||
| :widths: 30 70 | ||
| :header-rows: 1 | ||
|
|
||
| * - Option | ||
| - Description | ||
| * - ``--output``, ``-o`` | ||
| - Output path for the migration plan YAML (default: ``migration_plan.yaml``) |
There was a problem hiding this comment.
The CLI reference here doesn't match the actual rvl migrate plan flags implemented in redisvl/cli/migrate.py (uses --schema-patch and --plan-out, not --patch and --output/-o). As written, users following this page will get "unrecognized arguments" errors. Please update the syntax/option tables/examples to match the real CLI (or add backward-compatible aliases in the CLI).
| Apply the migration: rvl migrate apply --plan {plan_out} | ||
| Validate the result: rvl migrate validate --plan {plan_out} | ||
| To cancel: rm {plan_out}""" | ||
| f"\nTo add more changes: rvl migrate wizard --index {plan.source.index_name} --patch schema_patch.yaml" |
There was a problem hiding this comment.
The CLI guidance printed after plan generation hard-codes --patch schema_patch.yaml, but wizard allows --patch-out to be customized and plan may be generated from a patch with a different filename. Consider printing the actual patch path when available (e.g., pass patch_out into _print_plan_summary for wizard), or avoid referencing a specific filename.
| f"\nTo add more changes: rvl migrate wizard --index {plan.source.index_name} --patch schema_patch.yaml" | |
| f"\nTo add more changes: rvl migrate wizard --index {plan.source.index_name} --patch <PATCH_FILE>" |
| Completed: 2 | ||
| - products_idx: succeeded (10:02:30) | ||
| - users_idx: failed - Redis connection timeout (10:05:45) | ||
|
|
||
| In Progress: inventory_idx | ||
| Remaining: 1 (analytics_idx) |
There was a problem hiding this comment.
Batch status examples use succeeded, but the batch executor/state models use status: success (and failed/skipped). Please update these examples to use the actual status strings so they match CLI output and generated YAML.
| { | ||
| "cells": [ | ||
| { | ||
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "# SQL-to-Redis Query Translation: Hands-On Exercises\n", | ||
| "\n", | ||
| "This notebook provides hands-on exercises for learning the new **SQLQuery** feature in RedisVL, which allows you to write familiar SQL syntax that automatically translates to Redis Search commands.\n", | ||
| "\n", | ||
| "## What You'll Learn\n", | ||
| "\n", | ||
| "1. How to use the `SQLQuery` class to write SQL-like queries\n", | ||
| "2. Three equivalent approaches for the same queries:\n", | ||
| " - **RedisVL Python API** - Using native query classes (`FilterQuery`, `VectorQuery`, etc.)\n", | ||
| " - **RedisVL SQL** - Using the new `SQLQuery` class with SQL syntax\n", | ||
| " - **Raw Redis FT.SEARCH** - The equivalent Redis Search command\n", | ||
| "3. Various query types: filtering, numeric ranges, text search, aggregations, and vector similarity\n", | ||
| "\n", | ||
| "## Prerequisites\n", |
There was a problem hiding this comment.
This notebook addition appears unrelated to the PR’s stated focus on rvl migrate CLI + migration docs, and it significantly increases the PR surface area. Please confirm it’s intended for this PR; otherwise, consider moving it to a separate PR (or updating the PR description to explicitly include SQLQuery exercises).
|
Closing in favor of restructured PR stack (Option A ordering). |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Adds the complete
rvl migrateCLI with all subcommands (wizard, async, batch) and comprehensive documentation including a user guide, concept docs, and API reference.This PR extends the basic CLI from PR #560 to include all advanced subcommands that depend on the wizard, async, and batch modules added in PRs #562-#564.
Usage
All commands available after this PR:
What is included
redisvl/cli/migrate.py): CompleteMigrateclass with all subcommands including wizard, async apply, and batch operations.docs/concepts/index-migrations.md: Concept guide explaining migration modes, supported changes, and architecture.docs/user_guide/how_to_guides/migrate-indexes.md: Step-by-step how-to guide with examples.docs/api/cli.rst: API reference for the CLI.docs/concepts/index.mdanddocs/user_guide/index.mdfor navigation.docs/user_guide/cli.ipynb): Updated to include migrate commands.PR Stack
feat/migrate-corefeat/migrate-executorfeat/migrate-asyncfeat/migrate-batchfeat/migrate-wizardfeat/migrate-cli-docsfeat/migrate-benchmarks