Be strict about capturing UC dependencies (disallow references or extra dot-separated components) - #6667
Conversation
CaptureUCDependencies parsed compound UC names (vector search index
Name, quality monitor OutputSchemaName, model service Parent) with
strings.SplitN, which split on the dots inside a ${resources...}
reference and produced nonsensical catalog/schema fragments. It only
worked by accident: the fragments matched nothing, so the name was
reconstructed unchanged.
Make reference handling explicit:
- splitUCName treats a ${...} reference as atomic so a reference in any
component no longer breaks parsing.
- resolveCatalog/resolveSchema leave a component that is already an
explicit reference untouched.
- findSchema normalizes a catalog reference back to its literal name
(via literalCatalogName), so a literal schema is still captured when
its catalog is written as a ${resources.catalogs.<key>.name} reference.
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
| // splitUCName splits a UC identifier on "." into at most n parts, like | ||
| // strings.SplitN, but treats a ${...} reference as atomic so the dots inside a | ||
| // reference (e.g. ${resources.catalogs.c.name}) do not create extra splits. | ||
| func splitUCName(name string, n int) []string { |
There was a problem hiding this comment.
do we need to parse? should we just skip the implicit linking if we detect references or other divergence from a.b.c format (like unexpected number of components)?
what extra cases we solve by having this parser (which may work differently from our actual variable parser)?
There was a problem hiding this comment.
we can use dynvar.ContainsVariableReference early inside Apply(), but then we miss out on
${references.catalogs.my_catalog.name}.${var.my_schema}.my_vs_index
which at the time of capture_uc_dependencies is
${references.catalogs.my_catalog.name}.my_schema.my_vs_index
but you could argue that if you're already using references, you should be using it for all components. there's pros & cons to either (better user experience vs less maintainance).
or other divergence from a.b.c format (like unexpected number of components)?
seems a bit restricting but works for the currently onboarded resources. SplitN works also for e.g.
field := "foobar/mycatalog.myschema.my-custom.name"
rest := strings.CutPrefix(field, "foobar/")
parts := strings.SplitN(rest, ".", 3)
// use parts[0] and parts[1]but we can revisit if a resource ever adopts that pattern
Integration test reportCommit: da87e82
Top 6 slowest tests (at least 2 minutes):
|
no longer customer facing
janniklasrose
changed the title
|
PR description seems out of date?
this is no longer here, right? |
|
@denik dropped AI summary, hand-write what's done |
3 participants
Follow-up to #6655.
CaptureUCDependenciesparsed compound UC names. We now bail early for every resource if a${resources.*}reference is still there - users are expected to either declare every UC component via reference or none.Parsing out the components is also strict, requiring the expected number of dot-separated values.