direct: resolve immutable fields during plan #4417
Conversation
denik
requested review from
andrewnester,
anton-107,
pietern and
shreyas-goenka
as code owners
pietern
left a comment
pietern
left a comment
There was a problem hiding this comment.
Very nice.
This will make it possible to refer to the name field for the postgres resources.
| @@ -0,0 +1,3 @@ | |||
| trace $CLI bundle deploy | |||
| trace update_file.py databricks.yml "comment1" "comment2" | |||
| trace $CLI bundle plan | contains.py '1 to change, 0 to delete, 2 unchanged' | |||
There was a problem hiding this comment.
Please add a comment explaining what you expect.
I think I can trace this to expecting the update for the volume to not trigger an update for the job.
bundle/direct/dresources/adapter.go
Outdated
| return a.resourceConfig | ||
| } | ||
|
|
||
| func (a *Adapter) IsImmutableField(path *structpath.PathNode) bool { |
There was a problem hiding this comment.
This can use a comment.
The "immutability" test is conditional on the resource not being recreated.
It's not clear what the path is that's being referred to, is it a path in the remote state or the "comparable state". I suspect we're dealing with the comparable state only, because of the reference to "recreate on changes".
There was a problem hiding this comment.
It's not clear what the path is that's being referred to, is it a path in the remote state or the "comparable state". > I suspect we're dealing with the comparable state only, because of the reference to "recreate on changes".
Good question.
The end goal is to have "comparable state" be a subset of "remote state". As such, any path that works for comparable state would work for remote state as well. (This is not the case for some resources, but it'll be fixed and enforced).
I'm not sure if there is a use case for field that is both
a) present only in remote state but not in local state
b) immutable
I'll rename the method to IsFieldInRecreateOnChanges() because that's less ambiguous.
There was a problem hiding this comment.
There is one for the Postgres APIs: https://docs.databricks.com/api/workspace/postgres/getproject
The "name" field is even annotated as "immutable". It cannot be set by the client.
There was a problem hiding this comment.
Good example. Let's address that in a follow up. I think we should have immutable resource config entry (with similar effect as recreate_on_changes with respect to this change).
Changes
If field is in recreate_on_changes resource setting then we assume it does not change unless the resource is recreated and can resolve it before deploying the resource (like we do with "id" field).
Why
More precise plan. In some cases this can downgrade 'recreate' to 'update' or 'update' to 'skip'.
Tests
New regression test.