-
Notifications
You must be signed in to change notification settings - Fork 0
Kit 562 1 day add support for detecting changes in underlying assets #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Kit 562 1 day add support for detecting changes in underlying assets #3
Conversation
81024f9 to
84ed7bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this up. A couple of notes, and this general comment:
Please just use the boilerplate that Pulumi provides, to establish the baseline diff functionality. Don't modify it, refactor it, or add any extra spin on it. Just import it as close to verbatim as possible, with the correct license attribution in place.
The object of this exercise is to get /exactly/ what we had before, with the addition of checking hashes of the files.
No more, no less.
From my bird's eye view the logic should just be:
- If the files that create uses change, then we should signal to pulumi that we need to delete before replace (see
DiffResponse). - If the update files change, we need to signal that there's a difference and let Pulumi do its thing.
- If the delete files change, we don't care: nothing will happen until the point in the time the delete happens.
provider/pkg/runner/sshdeployer.go
Outdated
| newProps := resource.NewPropertyMap(newInput) | ||
|
|
||
| // Remove fields we want to ignore from the diff | ||
| delete(newProps, "connection") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would we be ignoring these? They weren't ignored by the previous boilerplate. What happens if I change the connection information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left over from my previous hand rolled approach using cmp to avoid panics.
provider/pkg/runner/sshdeployer.go
Outdated
| return fmt.Errorf("command is empty") | ||
| } | ||
|
|
||
| state.Command = def |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we doing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left over from early attempt to keep track of the previous command run to determine if an update occurred.
Add Pulumi compatible defaultDiff() Add Payload hashes to state Add last Command to state
84ed7bd to
1e19ea7
Compare
No description provided.