mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-07-06 02:52:11 +02:00
schema-lint chassis v1.2: --allow-data-loss flag + Hard mode (MR-694) — completes v1 (#100)
* schema-lint v1 commit 5: --allow-data-loss flag + Hard mode Final v1 commit. Wires up the --allow-data-loss CLI flag and Hard mode for both DropProperty and DropType. Per docs/dev/schema-lint-v1-plan.md, commit #5 of the schema-lint chassis v1 series (MR-694). CLI (omnigraph-cli/src/main.rs): - New --allow-data-loss flag on both `omnigraph schema plan` and `omnigraph schema apply` subcommands. Off by default (Soft). - HTTP remote schema apply explicitly rejects the flag for now (CLI-only; HTTP parity is a separate small follow-up that adds the field to SchemaApplyRequest + the server handler). Engine (omnigraph.rs + schema_apply.rs): - New SchemaApplyOptions { allow_data_loss: bool } public struct (Default = all false), re-exported via omnigraph::db::SchemaApplyOptions. - New public methods: plan_schema_with_options and apply_schema_with_options. Existing plan_schema/apply_schema are now thin wrappers that pass Default::default(). - promote_drops_to_hard: post-plan walk that promotes every DropMode::Soft step to DropMode::Hard when the flag is set. Keeps the compiler's plan_schema_migration signature unchanged (no breaking change for tests / callers). - Apply path: both Drop arms accept Hard mode; behavior is identical to Soft inside the apply loop. The DIFFERENCE is the new hard_cleanup_targets: Vec<(String, String)> accumulator, populated for every Hard variant with (table_key, full_dataset_uri). - Post-publish cleanup: a new loop after the manifest commit iterates hard_cleanup_targets and calls cleanup_old_versions (before_timestamp = now) on each dataset URI. Best-effort — the apply is already durable; cleanup failure is logged via tracing::warn rather than failing the apply. - New cleanup_dataset_old_versions helper inlines the Lance cleanup_old_versions call against a dataset URI. Behavioral details: - DropProperty Hard: stage_overwrite produced a new dataset version without the column. cleanup_old_versions removes the prior version (and reclaims unique fragments). After Hard apply, snapshot_at_version(pre_drop).open(table_key) FAILS because the prior dataset version was reclaimed. - DropType Hard: no per-table write happens (the change is the manifest tombstone). cleanup_old_versions on the orphan dataset is a no-op in the immediate term (no prior versions to clean since the dataset wasn't modified by this apply). The dataset directory persists. Full orphan-cleanup is a documented follow-up — the user-facing contract is "data is unreachable via omnigraph" (manifest entry tombstoned), which is satisfied. Tests (tests/schema_apply.rs): - apply_schema_with_allow_data_loss_promotes_drops_to_hard: default plan emits Soft; with options.allow_data_loss=true, plan emits Hard; apply succeeds. - apply_schema_hard_drops_property_makes_prior_version_unreachable: Hard drop succeeds, current snapshot lacks the column, and snapshot_at_version(pre_drop).open("node:Person") FAILS (Lance prior version reclaimed by cleanup). - apply_schema_hard_drops_node_and_edge_with_flag_succeeds: both Node and Edge DropType variants are promoted to Hard with the flag; apply succeeds; current manifest entries gone. (Orphan dataset directory cleanup deferred.) Test results: - cargo test -p omnigraph-compiler --lib: 239 passed - cargo test -p omnigraph-engine --test schema_apply: 14 passed (3 new Hard tests + 11 existing soft/regression tests) - cargo test -p omnigraph-server --test openapi: 60 passed (no HTTP API surface changes in this commit; OpenAPI parity follow-up noted) v1 status: complete for CLI/embedded use. MR-694 chassis epic + MR-700 DropType/DropProperty ticket can close after this lands. Known follow-ups (separate small PRs): - HTTP parity: extend SchemaApplyRequest with allow_data_loss field, thread through server handler, regenerate openapi.json. - Orphan-dataset directory deletion for DropType Hard (currently the dataset directory persists; cleanup_old_versions doesn't remove it because the dataset wasn't modified). - MR-948 substrate alignment: swap DropProperty Soft from stage_overwrite to Dataset::drop_columns (catalog_only vs full_rewrite cost class). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fixup: use bail! from color_eyre::eyre instead of anyhow The remote-rejection branch in SchemaCommand::Apply used anyhow::anyhow! which isn't in scope; the CLI's Result type is color_eyre::eyre::Result and bail! is already imported. Caught by CI Test Workspace job on PR #100. --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Andrew Altshuler
GitHub
parent
58cee158d8
commit
a6e037547f
5 changed files with 423 additions and 40 deletions
|
|
@ -316,6 +316,11 @@ enum SchemaCommand {
|
|||
schema: PathBuf,
|
||||
#[arg(long)]
|
||||
json: bool,
|
||||
/// Show the plan as it would execute with `--allow-data-loss`.
|
||||
/// Promotes every `DropMode::Soft` step to `DropMode::Hard`
|
||||
/// so the plan output reflects the destructive intent.
|
||||
#[arg(long, default_value_t = false)]
|
||||
allow_data_loss: bool,
|
||||
},
|
||||
/// Apply a supported schema migration
|
||||
Apply {
|
||||
|
|
@ -329,6 +334,17 @@ enum SchemaCommand {
|
|||
schema: PathBuf,
|
||||
#[arg(long)]
|
||||
json: bool,
|
||||
/// Allow destructive (data-loss) schema changes.
|
||||
///
|
||||
/// Without this flag, drops are "soft": the column or table
|
||||
/// is removed from the current manifest version but prior
|
||||
/// versions are retained, so `snapshot_at_version(pre_drop)`
|
||||
/// can still read the dropped data until `omnigraph cleanup`
|
||||
/// runs. With this flag, drops are "hard": `cleanup_old_versions`
|
||||
/// runs on the affected datasets immediately after the apply,
|
||||
/// making the prior data unreachable.
|
||||
#[arg(long, default_value_t = false)]
|
||||
allow_data_loss: bool,
|
||||
},
|
||||
/// Show the current accepted schema source
|
||||
#[command(alias = "get")]
|
||||
|
|
@ -1980,12 +1996,18 @@ async fn main() -> Result<()> {
|
|||
config,
|
||||
schema,
|
||||
json,
|
||||
allow_data_loss,
|
||||
} => {
|
||||
let config = load_cli_config(config.as_ref())?;
|
||||
let uri = resolve_local_uri(&config, uri, target.as_deref(), "schema plan")?;
|
||||
let schema_source = fs::read_to_string(&schema)?;
|
||||
let db = Omnigraph::open(&uri).await?;
|
||||
let plan = db.plan_schema(&schema_source).await?;
|
||||
let plan = db
|
||||
.plan_schema_with_options(
|
||||
&schema_source,
|
||||
omnigraph::db::SchemaApplyOptions { allow_data_loss },
|
||||
)
|
||||
.await?;
|
||||
let output = SchemaPlanOutput {
|
||||
uri: &uri,
|
||||
supported: plan.supported,
|
||||
|
|
@ -2004,6 +2026,7 @@ async fn main() -> Result<()> {
|
|||
config,
|
||||
schema,
|
||||
json,
|
||||
allow_data_loss,
|
||||
} => {
|
||||
let config = load_cli_config(config.as_ref())?;
|
||||
let bearer_token =
|
||||
|
|
@ -2011,6 +2034,12 @@ async fn main() -> Result<()> {
|
|||
let uri = resolve_uri(&config, uri, target.as_deref())?;
|
||||
let schema_source = fs::read_to_string(&schema)?;
|
||||
let output = if is_remote_uri(&uri) {
|
||||
if allow_data_loss {
|
||||
bail!(
|
||||
"--allow-data-loss is not yet supported on remote (HTTP) schema apply; \
|
||||
use `omnigraph schema apply` against a local path or s3:// URI for now"
|
||||
);
|
||||
}
|
||||
remote_json::<SchemaApplyOutput>(
|
||||
&http_client,
|
||||
Method::POST,
|
||||
|
|
@ -2023,7 +2052,13 @@ async fn main() -> Result<()> {
|
|||
.await?
|
||||
} else {
|
||||
let mut db = Omnigraph::open(&uri).await?;
|
||||
schema_apply_output(&uri, db.apply_schema(&schema_source).await?)
|
||||
let result = db
|
||||
.apply_schema_with_options(
|
||||
&schema_source,
|
||||
omnigraph::db::SchemaApplyOptions { allow_data_loss },
|
||||
)
|
||||
.await?;
|
||||
schema_apply_output(&uri, result)
|
||||
};
|
||||
if json {
|
||||
print_json(&output)?;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue