feat: Added SQL registry schema_mode and registry create command - #6704

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 67.39130% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.76%. Comparing base (b7ae488) to head (548ed0b).

@@            Coverage Diff             @@
##           master    #6704      +/-   ##
==========================================
+ Coverage   46.75%   46.76%   +0.01%     
==========================================
  Files         414      415       +1     
  Lines       50348    50392      +44     
  Branches     7208     7214       +6     
==========================================
+ Hits        23539    23568      +29     
- Misses      25157    25171      +14     
- Partials     1652     1653       +1     

@AdityaPatil22 Thanks for this — it's a focused, backward-compatible change that fills a real gap (running the SQL registry against a database where the runtime role shouldn't hold DDL grants). Overall looks good; a few minor points below, none blocking.

Strengths

• Default auto preserves existing behavior — no migration needed for current users.
• FeastRegistrySchemaError is actionable: it names the missing tables and the remediation command.
• The CLI engine construction mirrors the write-engine build (path + sqlalchemy_config_kwargs), so the pre-created schema matches what the runtime expects.
• Good docs coverage (yaml reference, CLI reference, dedicated SQL-registry section), and idempotency is called out.

Suggestions (minor / follow-up)

1. verify checks table existence only, not columns. _verify_schema compares table-name sets. A schema created by an older Feast version (missing a newer column) would pass verify but fail at query time. _warn_if_narrow_blob_columns already inspects column metadata, so there's precedent — worth either a doc caveat that verify is table-level only, or a follow-up that also checks expected columns.
2. verify skips the read replica. init only calls _verify_schema(self.write_engine). When read_path points at a separate replica, a missing table there won't be caught. The existing code runs _warn_if_narrow_blob_columns against both engines — mirroring that (verify the read engine too when read_engine is not write_engine) would be consistent.
3. teardown() still issues DDL. The stated motivation is "the app doesn't need DDL privileges at runtime," but teardown() drops tables regardless of schema_mode. That's defensible since teardown is an explicit admin action, but a one-line doc note would prevent surprise for a verify/skip deployment whose runtime role genuinely lacks DROP.
4. test_schema_mode_skip_does_not_run_ddl doesn't fully prove the claim. It pre-creates the schema, so it can't distinguish skip from auto/verify. A stronger assertion would run skip against an empty DB and assert it (a) does not raise and (b) leaves the DB with no registry tables — that directly proves "no DDL, no verification."

Nits

• The new from sqlalchemy import (inspect as sa_inspect,) is a separate import block directly below the existing large from sqlalchemy import (...) — worth folding in.
• FeastRegistrySchemaError.init(self, missing_tables: list[str]) uses list[str] while the rest of the file uses typing.List. Cosmetic, but matching the file convention would be nice.
• The test tempfiles from mkstemp() are never removed — teardown() drops the tables but the sqlite file leaks in the temp dir. Consider cleanup or a tmp_path fixture.

@nquinn408 Thanks for the thorough review!

Addressed all points:
verify checks table existence only, not columns - Added a doc caveat to the verify row in the mode table noting it's table-level only and that a schema from an older Feast version may pass verification but fail at query time. Column-level verification can be a follow-up.

verify skips the read replica - _verify_schema now runs against read_engine too when it differs from write_engine, mirroring the existing _warn_if_narrow_blob_columns pattern.

teardown() still issues DDL - On closer inspection, teardown() actually runs DELETE FROM (DML), not DROP TABLE (DDL), so it doesn't need DDL privileges. Updated the doc to correct this pre-existing inaccuracy: "the registry deletes all rows from the registry tables (it does not drop the tables themselves). This runs regardless of schema_mode and requires only DML (DELETE) privileges, not DDL."

test_schema_mode_skip_does_not_run_ddl - Rewritten to mock metadata.create_all and SqlRegistry._verify_schema, asserting neither is called. This directly proves the "no DDL, no verification" claim — the old test pre-created the schema so it couldn't distinguish skip from auto/verify.

Nits:

• list[str] → List[str] to match the file's typing.List convention.
• Folded inspect as sa_inspect into the main sqlalchemy block; ruff re-separates it because the alias can't share the # type: ignore comment - this is as merged as the linter allows.
• All tests switched from tempfile.mkstemp() to pytest tmp_path fixture for automatic cleanup.

@nquinn408 - I have updated the doc as suggested

@AdityaPatil22 Where are we with this PR? There is a test failure. Thanks for your work on this!