Repository navigation
Conversation
github-actions
Bot
added
DABs
NewPattern builds a pattern from parts like NewPath does for paths, so callers don't nest the single-step constructors or parse pattern strings. Co-authored-by: Isaac <no-reply@databricks.com>
…r refs and locations
Bundle configuration is the typed struct plus structvar (pure ${...}
references in non-string fields and a tree of source locations). YAML and
JSON decode straight into the structs; mutators, validators, variable
resolution, configsync and generate work on structvar views and structpath
paths. libs/dyn is deleted.
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
… test Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
- configsync: compare int and int64 values as equal when restoring variable references (variables decode as int, remote fields are int64). - schema annotations: merge object/sequence defaults as documented; accept an empty annotations file. - python mutator: use ResourceKeySet.AddPattern instead of a duplicate helper. - structvar.Set: don't rewrite an unchanged leaf location node (avoids cloning every ancestor per write). - ResolveConfigAgainstState: resolve all references in one call so lookups are shared. Co-authored-by: Isaac <no-reply@databricks.com>
…ctpath Co-authored-by: Isaac <no-reply@databricks.com>
- apply_default_task_source: an explicit source: "" is user-set; detect it by its definition location (not every task type has ForceSendFields). - pipeline_development_deprecated: warn on any user-written development value, including a reference resolving to an unset field. - rewriteShorthands: a later merged map takes precedence over an earlier one, and merge keys are expanded before detecting a full variable definition. - required: look dashboards up by a path that round-trips keys with path syntax. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
…om parts Earlier rebases resolved conflicts with #6980 in favor of string paths, reverting call sites that main already builds with structpath.NewPath. Restore main's versions where this PR has no reason to change them, and make the app permission checks take a path node instead of parsing a string. Co-authored-by: Isaac <no-reply@databricks.com>
… parts Root's location lookups take a *structpath.PathNode and say which value they return: NearestLocations/NearestLocation fall back to the closest ancestor with a location (for diagnostics), ExactLocations/ExactLocation return where the value is written (empty if a mutator set it). The string variants are gone, so callers build paths from parts instead of formatting and parsing them. Literal MustParsePattern/MustParsePath calls in production code are replaced with NewPattern/NewPath, and the app permission checks read the resource view directly. Co-authored-by: Isaac <no-reply@databricks.com>
IsForceSend boxed the ForceSendFields slice on every call and FieldValues built a slice per struct; both now run on every View traversal rather than once per conversion. Iterate with reflect and yield the fields instead. BenchmarkWalkReadOnly (10k jobs): 7.16M -> 4.77M allocs. Co-authored-by: Isaac <no-reply@databricks.com>
… fields Walking a configuration examines every field of every struct, so the per-field cost dominates: - eachField/eachMapItem/eachElem take a callback instead of returning an iterator, so walking no longer allocates closures per node (MapItems and Sequence wrap them). - StructInfo keeps a field list with the index, Go name, omitempty, the governing ForceSendFields and whether the key is a dotted path component, so a visit does no lookups by name; GetStructInfo is lock-free. - ForceSendFields are read once per struct visit, not once per zero field. - An unset field that cannot be present (not a struct or interface, no location, no reference) is skipped without building its view. BenchmarkWalkReadOnly (10k jobs): 316ms -> 183ms, 4.77M -> 1.58M allocs. Co-authored-by: Isaac <no-reply@databricks.com>
…ntation Co-authored-by: Isaac <no-reply@databricks.com>
Required and Enum walked the whole configuration and matched every node against ~580 generated patterns. The generator now emits walkers instead of pattern lists: they descend only along the paths that have rules, with plain field access, and check each node's fields in one visit. Equal subtrees share a function, so the walkers cover exactly the paths of the patterns. A string field is set if it is not empty or is in ForceSendFields; other fields also count as set if they hold a pure variable reference. References and locations are followed alongside the walk, so neither needs a path lookup. Co-authored-by: Isaac <no-reply@databricks.com>
Required fields have no omitempty, so they are never in ForceSendFields: an explicit zero (job_id: 0) or an empty block (run_job_task:) is only seen in the locations. Check those as the reflective walk did. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
denik
force-pushed
the
denik/typed-config-walkers
branch
from
October 18, 2026 06:43
cb28c17 to
b955c4d
Compare
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: e83880d
Top 6 slowest tests (at least 2 minutes):
|
The path visitors used by TranslatePaths and NormalizePaths iterate the typed resources with plain field access and pass the value as a *string, so the consumers rewrite it in place instead of looking the path up again with Set. Rules are applied one at a time over sorted resource keys, as the patterns were, so the visit order is unchanged. Paths are only built for the values that are visited. 1000 jobs (main / #6954 / this): TranslatePaths 384 / 28 / 8 ms, NormalizePaths 395 / 50 / 26 ms. Co-authored-by: Isaac <no-reply@databricks.com>
walk.Strings visits the strings of the configuration with plain field access
(generated, one function per struct type that contains strings), filtered
inline: WithReferences passes the strings that contain "${". The callback
can return a replacement, which is stored in the field. RewriteWorkspacePrefix,
TFOnlyReferences and reverse interpolation use it instead of walking every
value of the configuration; pure references in non-string fields come from
Root.References.
1000 jobs (main / #6954 / this): RewriteWorkspacePrefix 114 / 19 / 1.8 ms,
TFOnlyReferences 91 / 19 / 1.9 ms.
Co-authored-by: Isaac <no-reply@databricks.com>
An explicit "" (e.g. a pipeline root_path: '') is translated like before; only empty values that are not in the source are skipped. Co-authored-by: Isaac <no-reply@databricks.com>
Iterate the resources of each supported type and read and append their permissions through one generic function over the permission types, which all share the PermissionFields struct, instead of a pattern traversal and reflection on the Permissions field. Co-authored-by: Isaac <no-reply@databricks.com>
- Required/Enum: a zero scalar counts as present if it is in ForceSendFields or, without omitempty, written in the source; a nil pointer, slice or map is present only if non-nil or a pure reference; a nil pointer is visited as an empty node only if forced, written without omitempty, or a reference. This is how View.field decides it. - Path visitors: an empty value is present only if it is in its struct's ForceSendFields (not because it has a location). - Enum sorts diagnostics with the path tie-breaker. - walk.Strings visits typed values held in any (maps, slices, structs). - ApplyBundlePermissions: permissions holding a pure reference are present. - Only the Required walker builds the tree of references. Co-authored-by: Isaac <no-reply@databricks.com>
Resources.IterateAllResources, IterateAllPermissions and IterateAllGrants are generated from the fields of config.Resources, so new resource types are covered. Permissions are exposed through PermissionList, since their types differ between resources but all have the same fields. ApplyBundlePermissions uses IterateAllPermissions instead of a switch over resource types. Co-authored-by: Isaac <no-reply@databricks.com>
… lookups GetLocationOf/GetLocationsOf keep their names from main (they fall back to the closest ancestor with a location); GetLocationOfExact/GetLocationsOfExact return where a value is written. Avoids renaming callers that already exist on main. Co-authored-by: Isaac <no-reply@databricks.com>
…iterators FixPermissions, the grants check of Required, state loading, telemetry, the direct plan, the terraform migration and the single-resource check of includes use Resources.IterateAll* (and the new IterateAllResourceStates, GetOrCreateState, IsZero, Root.ResourceNodes) instead of pattern traversal. IterateAllResources now also visits resources defined as null, as the pattern did. PermissionList gains Set. Co-authored-by: Isaac <no-reply@databricks.com>
ConfigureClusterPolicyDefinition, ConfigureDashboardSerializedDashboard, ConfigureGenieSpaceSerializedSpace, InitializeVolumePaths, ExpandPipelineGlobPaths, ComputeIdToClusterId, PrependWorkspacePrefix and RewriteSyncPaths read the typed resources and fields directly instead of traversing patterns and views. Inline JSON documents are converted by one helper (inlineJSON). Co-authored-by: Isaac <no-reply@databricks.com>
…hecks config.Resources gains standard iterators over the places that hold libraries (task and for_each_task libraries, cluster libraries, job and pipeline environment dependencies, AI Runtime code sources). ExpandGlobReferences, the remote path collection, the duplicate library name check and the artifact glob expansion use them (and the typed artifacts) instead of pattern traversal and reflection on the sequences. Co-authored-by: Isaac <no-reply@databricks.com>
- Resource defaults move to bundle/config/resourcedefaults; the generator emits a typed walker (resourcemutator/defaults_walk.go) that visits only the paths with defaults and sets the unset ones, instead of one pattern traversal per default. bundle.SetDefaults is gone. A nil parent block (schedule: null) is not materialized, as on main. - SingleNodeCluster reads the cluster specs directly (clusters, job clusters, task and for_each_task clusters, pipeline clusters). - walk.Forced and walk.IsZeroStruct are shared by the generated walkers; Root.LocationTree exposes the locations tree. Co-authored-by: Isaac <no-reply@databricks.com>
Top level fields and resource groups come from the locations tree, resources and job tasks from the typed configuration, instead of a pattern traversal of views. SelectTarget deletes targets and environments (with their locations) rather than only clearing them. Co-authored-by: Isaac <no-reply@databricks.com>
ResourceKeySet.AddResources/AddResourcesOfType iterate the typed resources instead of matching a pattern over a view; the backwards-compatibility copy of 'python' to 'experimental.python' is a field assignment plus the locations subtree. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
…ngs walker The scope (whole configuration with or without resources, resources only, variable lookups) selects a walk.Strings entry point instead of a pattern matched over a view; the reflective string walk in hasReferencesToResolve goes away. Resolution itself still looks up arbitrary references in the view. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Replace View lookups and reflective walks where the types are known: - validate_engine, secret value, auth interpolation: plain field access with the presence rules (ForceSendFields, written, references); - environments -> targets: field assignment plus Root.MoveMetadata; - apps: Resources.PermissionsOf; plan prevent_destroy: Resources.GetResource (JobRun gets the GetLifecycle override it was missing); - DropEmptyStrings, CollectEscapeTelemetry: walk.Strings modes EmptyFields and NonEmpty; AllResourcesHaveValues: IterateAllResources; - selectResources: Resources.Select/CopyResource plus Root.CopyMetadata; - direct extractReferences and ResolveConfigAgainstState: walk.ResourceStringsOf plus Root.ReferencesUnder; - SetVariables iterates the typed variables and copies the default from a view of the field. The resources iterator generator is now a text/template. Co-authored-by: Isaac <no-reply@databricks.com>
- the python -> experimental.python copy carries references and copies the slices (Root.CopyMetadataAt; MoveMetadata uses it); - MergeGrants uses IterateAllGrants; - keyed merges and reference checks take built paths (IsReferenceAt); Root.IsReference(string) is gone; - ReferencesUnder(nil) covers every reference; - SingleNodeCluster no longer stores a context; - resolveIn is a regular function; - defaults_walk.go is registered as generated; - remove structvar.ForEach and structpath.MustParsePattern, now unused. Co-authored-by: Isaac <no-reply@databricks.com>
Locations are for diagnostics and path resolution. View, the generated
required/enum/defaults walkers and the mutators no longer treat a value
with a location as present; a value is present if it is not zero, is
forced, or holds a pure reference. Decoding records written zero values
in ForceSendFields, so they stay present where the struct has one.
Decisions:
- Required: an empty string never satisfies a required field, even if
forced; a forced false/0 does (zero is meaningful for bools and numbers).
So name: "" warns, and job_id: 0 in run_job_task warns (RunJobTask has
no ForceSendFields; a job id 0 is invalid anyway).
- Defaults keep a forced "" (the user asked for an empty value).
- Lost: a written zero in a struct without ForceSendFields and an empty
struct value (resources: {} -> summary .resources is null); an explicit
source: "" on spark_python_task/sql_task.file (no ForceSendFields) gets
the GIT default; development: true under mode: development no longer
warns (indistinguishable from the preset).
- workspace.file_path with immutable folders is rejected in
DefineDefaultWorkspacePaths, before the default is set, instead of by
checking its location.
- Dashboard required checks report by paths, not by locations found.
Co-authored-by: Isaac <no-reply@databricks.com>
Revised decision for required fields: a string is set if it is non-empty or forced, like every other scalar. An empty string is a valid value (the built-in SQL template has a job parameter whose default resolves to ""). A written zero in a struct without ForceSendFields cannot be told apart from unset, so it is reported missing (job_id: 0 in run_job_task warns); JobParameterDefinition.default is exempt because an empty default is valid. Also: - a struct whose only set fields are references is present (Node.HasRefs), in required checks and in the required/defaults traversal; - workspace.file_path with immutable folders is rejected in TranslatePaths again, after variable resolution, by value: any value other than the two defaults DefineDefaultWorkspacePaths sets is the user's; - remote_path and StateToBundle no longer use locations for presence; Resources.IsZero is gone; - resolve_config builds the state key from the path; - SingleNodeCluster checks clusters as it visits them, ctx as an argument. Co-authored-by: Isaac <no-reply@databricks.com>
… slices Most mappings are small: search their keys linearly and index only from 16 keys. Keep the key location as a value and build the slice only for a diagnostic. Load allocates 11% fewer bytes and 13% fewer objects. Co-authored-by: Isaac <no-reply@databricks.com>
- Root.References/ReferencesUnder skip references whose path is no longer
in the typed value, e.g. below a new_cluster that OverrideCompute cleared
for bundle.cluster_id; deploying used to fail resolving them ("parent is
nil pointer"). References is ReferencesUnder(nil).
- An explicit libraries: null on a pipeline (forced) errors again, as on main.
- Reference keys are rebased with path operations, not string concatenation.
Co-authored-by: Isaac <no-reply@databricks.com>
structaccess.Get treats a terminal nil pointer as reachable and skips
readonly/internal fields, so a new_cluster: ${var.cluster} that
OverrideCompute cleared came back, and bundle.terraform: ${var.x} was
dropped. A reference is now kept if the view finds its path: a nil
pointer (decoding allocates one for a reference) hides it.
Co-authored-by: Isaac <no-reply@databricks.com>
- A reference is kept only if the view shows it (a string): a forced nil pointer, e.g. a schedule: null that a target override referenced and a mutator then cleared, hides it too. The acceptance test covers the scenario (the test server does not reproduce the stale reference). - Set no longer adds an empty location node to mark a value present: presence does not use locations, and it cloned every ancestor on each default write. Co-authored-by: Isaac <no-reply@databricks.com>
- The generated strings walker does not descend into a field that holds a
pure reference, as the view does: with tags: ${...} and preset tags added
to the typed map, the hidden entries no longer become dependencies (a job
referred to itself and planning failed with a cycle).
- The immutable-folder file_path check cleans the value: a root_path ending
in / resolves the default to root//files.
- Scalars in dynamic values are not copied reflectively by the strings walk.
Co-authored-by: Isaac <no-reply@databricks.com>
- Every generated struct walker returns when its node holds a pure reference, so a referenced map entry, slice element or resource (also in the per-resource dispatch) hides what is below it, as in the view. - Scoped walks (resources, one resource, variable lookups) build the reference tree from the references in their scope only. Co-authored-by: Isaac <no-reply@databricks.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Replacement for #6954 (not stacked on it; shares its commits): libs/dyn is removed and the bundle configuration is typed structs with
structvarreferences and locations. On top of that, every traversal of the typed configuration is plain struct access:bundle/internal/validation,go run ./bundle/internal/validation):validate.Required/validate.Enum(only the paths that have rules), resource defaults, a strings walker (modes: strings with${, non-empty strings, forced empty fields), and resource iterators inconfig(IterateAllResources/Permissions/Grants/ResourceStates,GetResource,PermissionsOf,Select,CopyResource). The resource iterator generator is atext/template.structvar.ForEachis gone; mutators use the standard visitors above.Viewis only used where data is dynamic (decode/merge/override, Python input/output, configsync, resolving arbitrary${...}paths, JSON output).ForceSendFieldsonly, never from locations. A value is present if it is non-zero, forced, or a pure reference; decoding records written zero values inForceSendFields. Decisions and what this loses:ForceSendFields(name: "",default: ""). In structs without them it cannot be told from unset, so it is reported missing (job_id: 0inrun_job_tasknow warns); a job parameter'sdefaultis exempt, as an empty default is valid.presets: {},resources: {}) is absent:bundle summary -o jsonhas noresourceskey forresources: {}.source: ""onspark_python_task/sql_task.file(noForceSendFields) gets theGITdefault on git_source jobs.developmentdeprecation warning is not shown fordevelopment: trueundermode: development(indistinguishable from the preset).Why
Every component should be faster than libs/dyn, not just the total. 1000 jobs, one
bundle.Applyeach (main pays ~90 ms of conversion around every mutator):Load is still slower (decoder intermediates); everything else is 10-1000x faster.
Tests
Unit tests; acceptance goldens changed only for the presence decisions above (net:
job_id: 0warns). Parity tests guarding old behaviour are in #7085.This pull request and its description were written by Isaac.