-
Notifications
You must be signed in to change notification settings - Fork 82
Remove the rolling configuration architecture #1185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,13 +6,11 @@ | |
| > into a *new* stable schema file at promotion time (per | ||
| > [Promoting to Stable](#promoting-to-stable) below). | ||
| > | ||
| > **Stable schemas document only the non-experimental surface.** | ||
| > Experimental backends, the `experimental.*` block, and any in-progress | ||
| > shapes live solely in `schemas/dev/` — they must not be mirrored into | ||
| > a stable file. Configs that need editor validation for experimental | ||
| > fields should point `$schema` at the dev file. The `--experimental` | ||
| > runtime gate is unchanged: it still controls execution regardless of | ||
| > which schema validated the config. | ||
| > **Development features use permanent field locations.** | ||
| > Add an in-progress backend or feature at its intended top-level location in | ||
| > the mutable development contract. Do not place it under an | ||
| > `experimental` JSON wrapper. The `--experimental` runtime gate controls | ||
| > execution until graduation regardless of which schema validated the config. | ||
|
|
||
| ## Prerequisites | ||
|
|
||
|
|
@@ -113,36 +111,35 @@ Adding a feature may touch these files: | |
| | File | What to change | | ||
| |------|----------------| | ||
| | `src/core/mxc_config_contract/src/dev/` | Add the field to the authoritative closed mutable development contract | | ||
| | `src/core/wxc_common/src/wire.rs` | Mirror the field in the rolling differential model while that characterization oracle remains | | ||
| | `src/core/wxc_common/src/config_contract_adapters/dev/` | Adapt the exact field into private `ConfigInput` | | ||
| | `src/core/wxc_common/src/wire.rs` | Add only reusable nested normalization DTOs needed by the adapter; never add a whole-request root | | ||
| | `src/core/mxc_engine/src/policy/exact/v0_10.rs` | If the Rust SDK exposes the field, update the production exact development builder | | ||
| | `schemas/dev/mxc-config.schema.0.10.0-dev.json` | **Generated rolling artifact** — do not hand-edit | | ||
| | `schemas/dev/mxc-config.schema.0.10.0-alpha.json` | **Generated exact artifact** — do not hand-edit | | ||
| | `sdk/node/src/generated/v0_10_0_alpha/wire.ts` | **Generated exact artifact** — do not hand-edit | | ||
| | `src/core/wxc_common/src/models.rs` | Add `GpuIsolationConfig` struct, add field to `ExperimentalConfig` | | ||
| | `src/core/wxc_common/src/config_parser.rs` | Map the new wire field to the domain struct in `convert_wire_config` | | ||
| | `src/core/wxc_common/src/config_parser.rs` | Map the new config-input field to the domain struct in `convert_config_input` | | ||
| | Runner (`appcontainer.rs` or `lxc_runner.rs`) | Feature logic, guarded behind `experimental_enabled` | | ||
| | `tests/configs/` | Test config exercising your feature | | ||
|
|
||
| ## Step 1: Add the field to the exact contract and rolling oracle | ||
| ## Step 1: Add the field to the exact contract and config input | ||
|
|
||
| Add the feature to the authoritative closed request types under | ||
| `src/core/mxc_config_contract/src/dev/`, then mirror it in the rolling Rust wire | ||
| model (`src/core/wxc_common/src/wire.rs`) while differential characterization | ||
| remains. The rolling experimental struct remains permissive; the exact | ||
| development contract and every nested experimental object are recursively | ||
| closed. | ||
| `src/core/mxc_config_contract/src/dev/`, then adapt it into the shared internal | ||
| config input used by semantic normalization. | ||
| The exact development request and every nested object are recursively closed. | ||
|
|
||
| ```rust | ||
| // in wire.rs | ||
| pub struct Experimental { | ||
| pub compartments: Option<Compartments>, | ||
| pub gpu_isolation: Option<GpuIsolation>, // ← add this | ||
| // ... | ||
| // in mxc_config_contract/src/dev/one_shot.rs | ||
| pub struct Request { | ||
| // Existing permanent fields... | ||
| #[serde(default)] | ||
| pub gpu_isolation: OptionalField<GpuIsolation>, | ||
| } | ||
|
|
||
| /// GPU device isolation (experimental). | ||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| #[derive(Debug, serde::Deserialize)] | ||
| #[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))] | ||
| #[serde(rename_all = "camelCase")] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
| pub struct GpuIsolation { | ||
| /// GPU device index to assign to the container. | ||
| pub device_index: Option<u32>, | ||
|
|
@@ -157,14 +154,12 @@ The `///` doc comments become schema `description`s and `#[schemars(...)]` | |
| attributes become constraints. Then regenerate the committed schema: | ||
|
|
||
| ``` | ||
| cargo run --manifest-path src/Cargo.toml -p mxc_schema_gen -- schema --legacy-wire --out schemas/dev/mxc-config.schema.0.10.0-dev.json | ||
| cargo run --manifest-path src/Cargo.toml -p mxc_schema_gen -- types --legacy-wire --out sdk/node/src/generated/wire.ts | ||
| cargo run --manifest-path src/Cargo.toml -p mxc_schema_gen -- schema --version 0.10.0-alpha --out schemas/dev/mxc-config.schema.0.10.0-alpha.json | ||
| cargo run --manifest-path src/Cargo.toml -p mxc_schema_gen -- types --version 0.10.0-alpha --out sdk/node/src/generated/v0_10_0_alpha/wire.ts | ||
| ``` | ||
|
|
||
| The rolling and exact codegen gates fail if any committed artifact drifts, so | ||
| all applicable regeneration steps are mandatory. | ||
| The exact codegen gate fails if either committed artifact drifts, so both | ||
| regeneration steps are mandatory. | ||
|
|
||
| ## Step 2: Add the model struct | ||
|
|
||
|
|
@@ -194,9 +189,9 @@ pub struct ExperimentalConfig { | |
|
|
||
| Production parsing first deserializes JSON into the exact registered request | ||
| contract. The version-specific adapter then converts that closed type into the | ||
| shared `wire::MxcConfig` representation used by semantic normalization. Add the | ||
| shared private `config_input::ConfigInput` used by semantic normalization. Add the | ||
| adapter mapping for your exact contract field, then map the corresponding wire | ||
| field to the domain struct inside `convert_wire_config`: | ||
| field to the domain struct inside `convert_config_input`: | ||
|
|
||
| ```rust | ||
| let experimental = if let Some(raw_exp) = cfg.experimental { | ||
|
|
@@ -223,9 +218,8 @@ Add tests to verify: | |
| - `gpuIsolation` is accepted by the exact request root and maps through its | ||
| adapter to `ExecutionRequest.experimental` | ||
| - Missing optional fields use defaults | ||
| - Unknown fields under exact `experimental` objects are rejected | ||
| - The rolling parser's permissive behavior remains characterized separately | ||
| while that differential oracle exists | ||
| - Unknown fields in the exact feature object are rejected | ||
| - Version-boundary tests reject the field from contracts that predate it | ||
|
|
||
| ## Step 4: Implement the feature in the runner | ||
|
|
||
|
|
@@ -332,25 +326,21 @@ The SDK passes `--experimental` to the underlying binary when this is set. | |
|
|
||
| When your experimental feature is ready to ship: | ||
|
|
||
| 1. Move the field from `experimental` to the top-level stable-candidate surface | ||
| in both transitional Rust models, then regenerate all rolling and exact | ||
| artifacts with `mxc_schema_gen` | ||
| 1. Carry the field from the mutable development contract into the next exact | ||
| stable contract at the same permanent location, then regenerate its exact | ||
| schema and TypeScript artifacts with `mxc_schema_gen` | ||
| 2. Move the struct from `ExperimentalConfig` to `ExecutionRequest` | ||
| 3. Map the now-top-level wire field in `convert_wire_config` (and add | ||
| `deny_unknown_fields` to the wire struct so the promoted, stable surface is | ||
| closed) | ||
| 3. Add the stable contract adapter mapping while retaining the existing | ||
| `convert_config_input` domain normalization | ||
| 4. Remove the `if request.experimental_enabled` guard | ||
| 5. Bump the minor version | ||
| 6. Add a parser error for configs still referencing the feature under | ||
| `experimental`: `"gpuIsolation has moved to the stable section"`. | ||
| This error should persist for at least one release cycle so users have | ||
| time to migrate, then it can be relaxed to the standard "unknown field" | ||
| behavior. | ||
| 6. Preserve every published contract unchanged; older contracts continue to | ||
| reject the field structurally. | ||
|
|
||
| ## Checklist | ||
|
|
||
| - [ ] Rolling and exact development contract types updated | ||
| - [ ] Rolling and exact generated schemas and TypeScript oracles regenerated | ||
| - [ ] Exact development contract and adapter updated | ||
| - [ ] Exact generated schema and TypeScript oracle regenerated | ||
|
Comment on lines
+342
to
+343
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: just an fyi, i think if rolling doesn't exist anymore then "exact" probably loses it's meaning right? and then we can call it "development contract" for example.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in #1188. Mostly agreed. I removed redundant phrases such as "exact development contract" where "development contract" or "next published contract" is sufficient. I retained "exact contract" where it distinguishes an exact registered version from range-based or rolling interpretation. |
||
| - [ ] Model struct added to `models.rs` | ||
| - [ ] Exact contract adapter and domain mapping added with unit tests | ||
| - [ ] `--experimental` flag wired through (if not already) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the domain struct? not mentioned in this file anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in #1188. Good catch. I replaced the undefined "domain struct" terminology with the concrete flow: the exact contract field is adapted through
CommonRequestIRand normalized intomodels::GpuIsolationConfigonExecutionRequest.