Read live Job state for replaceUnready check #1578
Conversation
The check added in de129e3 mistakenly did not check the live status of the Job, but instead checked the user inputs. This would lead to erroneous replacements for ready Jobs.
Does the PR have any schema changes?Looking good! No breaking changes found. |
1 similar comment
Does the PR have any schema changes?Looking good! No breaking changes found. |
| hasChanges = pulumirpc.DiffResponse_DIFF_SOME | ||
| replaces = append(replaces, `.metadata.annotations["pulumi.com/replaceUnready"]`) | ||
| // Fetch current Job status and check point-in-time readiness. Errors are ignored. | ||
| if live, err := k.readLiveObject(newInputs); err == nil { |
There was a problem hiding this comment.
Extending from @lukehoban's comment here: #1575 (comment)
should we be more fail-fast with error handling in providers in general? We don't have very good means of surfacing "warning" type diagnostics to the CLI so I wonder if we should bubble up the errors here rather than swallow them and result in surprising behavior for the user?
There was a problem hiding this comment.
In this particular case, the fallback behavior is a regular update, which would fail in the usual manner in case of a persistent error. Since this is more conservative than replacing, I'm inclined to just ignore the error.
Proposed changes
The check added in de129e3 mistakenly did not check the
live status of the Job, but instead checked the user inputs.
This would lead to erroneous replacements for ready Jobs.
Related issues (optional)