Conversation
jshearer
force-pushed
the
jshearer/target_naming
branch
2 times, most recently
from
6ae3c5a to
a9348ac
Compare
`TargetNaming` lives inside `SourceDef`, so materializations without a sourceCapture cannot express a naming preference, and the string-enum variants carry no schema parameter. This adds `target_naming: Option<TargetNamingStrategy>` directly on `MaterializationDef` with three variants: `matchSourceStructure`, `singleSchema { schema }`, and `prefixTableNames { schema, skipCommonDefaults }`.
* All variants require the connector to support `x-schema-name` in its resource config schema. The old `SourceDef.target_naming` is preserved as a legacy fallback for unmigrated specs.
* On publish, `source.targetNaming: withSchema` is auto-promoted to `targetNaming: matchSourceStructure`
jshearer
force-pushed
the
jshearer/target_naming
branch
from
a9348ac to
ba73cac
Compare
| if target_naming.is_none() { | ||
| if let Some(source) = &sources { | ||
| if source.to_normalized_def().target_naming == models::TargetNaming::WithSchema { | ||
| target_naming = Some(models::TargetNamingStrategy::MatchSourceStructure); | ||
| model_fixes.push( | ||
| "promoted source.targetNaming 'withSchema' to top-level targetNaming 'matchSourceStructure'".to_string(), | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
am I right that this is going to still retain source.targetNaming while adding the new top-level targetNaming? Can we get rid of the old one automatically as well?
There was a problem hiding this comment.
The intent is that it will leave source.targetNaming alone specifically in order to allow this to be progressively released. For example, in update_materialization_resource_spec() we first check if the new targetNaming exists, then fall back to the original check against source.targetNaming.
The intent is to allow this PR to be released, then the UI to be updated to use the new targetNaming, though I'm realizing as I write that that we actually need to split this up even further: we need to introduce the new targetNaming without even this trivial upgrade, then the UI needs to be updated to handle it, then we can introduce the model fix(es). Reason being, otherwise we would do this one-time upgrade, but the UI would continue to use source.targetNaming.
I will remove this model fix until the UI is updated
|
I have not yet fully taken a look here or messed with it locally. But want to know about the auto promotion during publish. If this literally moves the setting our of Also - we renamed that prop a bit ago - so we might also want to promote |
We need to ship the pure model update so that clients can be updated to use it progressively (i.e the UI should present and edit it if present, otherwise it should use the existing field)
|
@travjenkins does this answer your question? The intent is to thread the needle of deploying this without breaking anything:
|
|
Okay - that generally tracks in my mind and basically how we did this before. And I will try to ensure the UI uses the new stuff in create all the time. The UI also previously migrated things if a user edited a task. We can chat about if we want to do that or not. |
3 participants
Summary
Phase 1 of the target naming redesign (#2780). This PR adds
TargetNamingStrategyas a new top-level field onMaterializationDef, decoupling naming behavior fromSourceDefso that materializations without a sourceCapture can express a naming preference.What changed
TargetNamingStrategyis a tagged-object enum with three variants:matchSourceStructure: schema from collection name, table is the final componentsingleSchema { schema }: all tables in one named schemaprefixTableNames { schema, skipCommonDefaults }: schema component prefixed to table nameThe existing
SourceDef.target_namingenum is preserved as a fallback. When bothMaterializationDef.target_namingandsource.target_namingare present, the top-level field wins for naming;delta_updatesis always read fromsource. The next phase of work is a migration to populate the remaining materializations' roottarget_namingbased on knowledge of each connector's endpoint-configschemalocation and default.Auto-promotion on publish
When a materialization has
source.targetNaming: withSchemaand no top-leveltarget_naming, the validation layer promotes it totargetNaming: { strategy: "matchSourceStructure" }as a model fix. This is the only variant that doesn't require extracting a schema value from the connector's endpoint config, so it's the only one promoted in this phase. Other variants (noSchema,prefixSchema,prefixNonDefaultSchema) remain onSourceDef.target_naminguntil Phase 2 extracts the endpoint config schema.WASM API
The
update_materialization_resource_specWASM function now acceptstargetNamingalongsidesourceCapture, both optional.sourceCaptureis optional because not all materializations have one, not because it's being replaced. Existing UI code that sendssourceCapturecontinues to work unchanged.Note on
flow.schema.jsonThe regenerated schema includes
Triggers,TriggerConfig, andHttpMethodtypes that were already in the models but missing from the previously-staleflow.schema.json. These are unrelated to this PR.