Expose computed volume_path during initialize#5550
Open
radakam wants to merge 14 commits into
Open
Conversation
Collaborator
Integration test reportCommit: b3d6786
21 interesting tests: 13 SKIP, 7 KNOWN, 1 RECOVERED
Top 4 slowest tests (at least 2 minutes):
|
6211576 to
0399701
Compare
3aa9504 to
b30a845
Compare
2827a4e to
a240d86
Compare
a240d86 to
3334a23
Compare
Contributor
Approval status: pending
|
82dfe46 to
8918aa0
Compare
Add a computed, read-only volume_path field to volume resources, set to
the Unity Catalog path /Volumes/{catalog}/{schema}/{name} during the
initialize phase. This enables other resources to reference
${resources.volumes.<key>.volume_path}.
- InitializeVolumePaths resolves catalog_name/schema_name/name references
locally to compute the path without mutating the original references, so
validate and plan keep showing the references.
- volume_path is only set when catalog, schema, and name resolve to
concrete values; unresolved or malformed references leave it empty.
- Computation runs after PythonMutator: volume_path is a computed field the
PyDABs Volume model does not declare, so it must not be exposed to Python.
Cover behaviors the initial volume_path change did not exercise: - unresolved_volume_path: a volume whose name is only known at deploy cannot have volume_path computed at plan time, so a reference to it resolves to an empty string (no error). Documented as known badness. - volume_path_job_ref: the motivating use case from #4233, a job parameter referencing ${resources.volumes.<key>.volume_path}. - Unit test for resolving a reference to an unset volume_path on an existing volume (resolves to empty rather than erroring).
Extend the volume_path_job_ref test to confirm ${resources.volumes.data.volume_path}
is interpolated end-to-end: record the JSON plan per engine and capture the
jobs/create request from a real deploy. Enable RecordRequests so the requests
can be inspected.
A volume component (catalog_name, schema_name, name) that cannot be
resolved locally is now embedded verbatim into volume_path as a ${...}
reference instead of suppressing the path. The embedded reference is
carried through ${resources.volumes.<key>.volume_path} interpolation and
resolved later by the engine during plan or deploy, like any other
resource reference.
Because volume_path is a readonly field that is dropped from state before
deploy, makePlan no longer treats a reference carried by such a field as a
dependency (the reference is still made available to readers during
initialize).
Rewrite unresolved_volume_path so an embedded ${...} reference in
volume_path is resolved at deploy rather than treated as an error: bar.name
comes from baz.id and foo.comment reads bar.volume_path, and the recorded
deploy requests confirm the path is fully interpolated. Update
non_existent_field output for the embedded-reference volume_path.
… trim comment Rename the volume_path acceptance fixture to better reflect what it covers, and shorten the explanatory comment in unresolved_volume_path.
The deploy order is deterministic via the dependency chain (schema -> bar -> foo, since foo.comment references bar.storage_location), so sorting only hid the ordering signal the test exists to verify.
8918aa0 to
d44f9f5
Compare
# Conflicts: # NEXT_CHANGELOG.md # bundle/terraform_dabs_map/generated.go
denik
reviewed
Jun 24, 2026
Pass the resource's state type into extractReferences and drop references to fields absent from state (e.g. volumes' computed volume_path) during the walk, instead of post-filtering the combined refs map. Validating the structpath we already build also avoids a string round-trip via structpath.ParsePath. Also rename the acceptance test unresolved_volume_path -> volume_path_contains_id and fix its script to invoke "$CLI bundle plan -o json".
Extend volume_path_job_ref so the volume's schema_name/name reference another job's remote creator_user_name, so volume_path embeds an unresolved reference at initialize that resolves at deploy. Add the same scenario as a no_drift invariant config.
# Conflicts: # NEXT_CHANGELOG.md # bundle/direct/bundle_plan.go
Rewrite the state-type filter in extractReferences as a positive check to satisfy the nilerr linter. Exclude the volume_path_job_ref invariant config from the migrate and continue_293 suites: volume_path is unsupported by the v0.293.0 seed CLI, and its embedded creator_user_name reference is not a terraform-exported attribute. It remains covered by no_drift on direct.
…nt semantics InitializeVolumePaths now rejects a user-provided volume_path instead of silently overwriting the computed value, and ResolveVolumePathReferencesOnlyResources errors when a referenced volume_path could not be computed rather than injecting an empty string. Add a set-volume-path acceptance test for the rejection and expand the changelog to document the deploy-ordering and Terraform-engine caveats.
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
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
Today, if you want to point one resource at a bundle-managed volume, you have to hand-write its full UC path:
This PR adds a computed, read-only
volume_pathon every volume so you can just reference it:${resources.volumes.my_volume.volume_path}volume_pathis/Volumes/{catalog}/{schema}/{name}and shows up invalidateandplan, so what you see is what gets deployed.How it works:
Volume.VolumePath+ComputeVolumePath()build the path fromcatalog_name,schema_name, andname. If a part is still an unresolved${...}reference, it's kept embedded and resolved later at plan/deploy; a malformed reference produces no path rather than leaking garbage.InitializeVolumePathscomputes the path duringinitialize. Whencatalog_name/schema_namepoint at${resources...}references, it resolves them only to build the path — your original references stay untouched in config. References that can't be resolved yet (e.g. a remote field like another resource'sid/creator_user_name) are embedded intovolume_pathand resolve at deploy.ResolveVolumePathReferencesOnlyResourcesthen resolves references tovolume_pathand nothing else.PythonMutator, sincevolume_pathis computed/read-only and isn't part of the PyDABs model (same treatment asdeploymentmetadata).alwaysSkipin config sync, and excluded as a direct-engine dependency. The exclusion happens inextractReferences, which validates each reference's field path against the resource's state type and drops fields (likevolume_path) that exist in input config but not in state.Fixes #4233.
Why
Hardcoded volume paths are duplicated across a config and silently break when a catalog, schema, or volume name changes. A symbolic
volume_pathkeeps the reference tied to the definition. This takes the schema + initialize-mutator approach so the path is visible atvalidate/plantime.Tests
ComputeVolumePath(resolved / embedded reference / malformed),InitializeVolumePaths, volume-path-only resolution, and the Terraform drop.computed_volume_path— a volume'scommentreferences another volume'svolume_pathand resolves, while the originalschema_namereference is left intact.volume_path_job_ref— a job parameter references a volume'svolume_path(the motivating use case from volumes: volume_path field is not available #4233). The volume'sschema_name/namethemselves reference another job's remotecreator_user_name, sovolume_pathis computed at initialize with the reference embedded and the whole chain (foo→data.volume_path→process) resolves at deploy. Records per-engine plan and deploy requests to confirm interpolation.volume_path_contains_id— a volume'snameis a remote value (another volume'sid), so itsvolume_pathcarries that reference, and a third volume'scommentpicks up the embedded string and resolves at deploy.change-comment,change-name,change-schema-name,bad_syntax,non_existent_field, …) now showing the new field.volume_path_job_refconfig that deploys the embedded-reference chain and asserts no drift on re-plan.