Expose computed volume_path during initialize#5550
Open
radakam wants to merge 18 commits into
Open
Conversation
Collaborator
Integration test reportCommit: e552008
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
|
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.
Update ExtractReferences doc to reflect state-type filtering of references, correct the InitializeVolumePaths interpolation comment to point at the resolver mutator, and note the run-exactly-once requirement.
The remote-field embedding scenario (volume_path carrying an after-deploy
${...creator_user_name}) is only supported on the direct engine, which is
the default. Recording Terraform's provider-level failure as expected output
was brittle, so run this case on direct only and drop the stale Terraform
artifacts. Correct the changelog: ${...volume_path} references resolve on both
engines when components are known at initialize; only after-deploy components
are direct-only.
Resolve conflicts: - resolve_variable_references.go: keep both allowPathFn (volume_path) and excludePaths (snapshot workspace paths) skip checks. - invariant out.test.toml: regenerate; drops stale volume_path_job_ref and volume_uppercase_name entries that the single-condition excludes remove.
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
Adds a computed, read-only
volume_pathtoresources.volumes.*so configs can use${resources.volumes.<key>.volume_path}instead of hardcoding/Volumes/<catalog>/<schema>/<name>(#4233).Volume.VolumePath(bundle:"readonly") +ComputeVolumePath().InitializeVolumePathsmutator: computes the path, rejects a user-set value.ResolveVolumePathReferencesOnlyResourcesmutator: resolves${...volume_path}referrers.extractReferencesnow gates references on the resource's state type (ripples throughExtractReferences,migrate/build_state.go).volume_pathinconvert_volume.go, remove fromterraform_dabs_map.Why
The path is fully determined by
catalog_name/schema_name/name, so deriving it removes duplication and makes a reference depend on those components, not the volume itself. It's computed atinitializeand never persisted (not sent to the API, dropped before Terraform apply), afterPythonMutatorsince the PyDABs model doesn't declare it. Unresolved components are embedded and resolved later; a reference in a readonly field can't resolve against state, hence theextractReferencesgating.Tests
Unit:
ComputeVolumePath, the two mutators, andextractReferencesdropping the readonly field.Acceptance:
computed_volume_pathandvolume_path_contains_id(resolution at validate/deploy, both engines);volume_path_job_ref(job-parameter case, direct-only since the embeddedcreator_user_nameis only known after deploy);set-volume-path(user-set value rejected).