Merged
Conversation
denik
requested review from
andrewnester,
anton-107,
pietern and
shreyas-goenka
as code owners
Collaborator
17 failing tests:
|
andrewnester
reviewed
Aug 4, 2025
| // TODO: Name, SchemaName changes should result in re-create | ||
| for _, change := range changes { | ||
| if change.Path.String() == ".name" { | ||
| return deployplan.ActionTypeUpdateWithID |
Contributor
There was a problem hiding this comment.
As an example jobs or pipelines allow changing the name but they don't implement DoUpdateWithId, how does it work with them?
Contributor
Author
There was a problem hiding this comment.
Discussed offline - this change is for volumes only.
"bundle deploy" no longer calculates and classifies the diff, it relies
on action types calculated during "bundle plan". (Note, this will need to
be further developed once we handle ${resources} references - we might want to
recalculate the diff in some cases.).
We no longer have UpdateUpdatesID setting on the resource. Instead,
resources can implement optional method DoUpdateWithID that can return an
updated id as part of the update. Currently, only volume uses that.
The original DoUpdate method no longer allows to return ID. By
convention, if possible, it also checks that ID is not changed by the
backend.
There is new ActionType: "update_with_id". This is what triggers
DoUpdateWithID() call. However, when stringified, this is presented as
"update" so that it's compatible with terraform output in "bundle plan".
The main motivation is to have information at plan-time on whether we're
going to update ID or not. If not, we can resolve ${resources.*.*.id}
right away and have more parallelism at deploy time.
denik
force-pushed
the
denik/refactor-deploy-plan
branch
from
eb4d356 to
b5d1309
Compare
andrewnester
approved these changes
Aug 5, 2025
alyssa-db
pushed a commit
that referenced
this pull request
Aug 7, 2025
## Changes
"bundle deploy" no longer calculates and classifies the diff, it relies
on action types calculated during "bundle plan". Note, this will need to
be further developed once we handle ${resources} references - we might
want to recalculate the diff in some cases.
We no longer have UpdateUpdatesID setting on the resource. Instead,
resources can implement optional method DoUpdateWithID that can return
an updated id as part of the update. Currently, only volumes uses that.
The original DoUpdate method no longer allows to return ID. By
convention, if possible, it also checks that ID is not changed by the
backend.
There is new ActionType: "update_with_id". This is what triggers
DoUpdateWithID() call. However, when stringified, this is presented as
"update" so that it's compatible with terraform output in "bundle plan".
## Why
The main motivation is to have information at plan-time on whether we're
going to update ID or not. If not, we can resolve ${resources.*.*.id}
right away and have more parallelism at deploy time.
Additionally, this simplifies usual case DoUpdate() that resources need
to implement.
## Tests
Existing tests. The new action type is tested by
#3349
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
3 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
"bundle deploy" no longer calculates and classifies the diff, it relies on action types calculated during "bundle plan". Note, this will need to be further developed once we handle ${resources} references - we might want to recalculate the diff in some cases.
We no longer have UpdateUpdatesID setting on the resource. Instead, resources can implement optional method DoUpdateWithID that can return an updated id as part of the update. Currently, only volumes uses that.
The original DoUpdate method no longer allows to return ID. By convention, if possible, it also checks that ID is not changed by the backend.
There is new ActionType: "update_with_id". This is what triggers DoUpdateWithID() call. However, when stringified, this is presented as "update" so that it's compatible with terraform output in "bundle plan".
Why
The main motivation is to have information at plan-time on whether we're going to update ID or not. If not, we can resolve ${resources...id} right away and have more parallelism at deploy time.
Additionally, this simplifies usual case DoUpdate() that resources need to implement.
Tests
Existing tests. The new action type is tested by #3349