-
Notifications
You must be signed in to change notification settings - Fork 57
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
feat: report status when stack is locked #807
Conversation
This change is part of the following stack: Change managed by git-spice. |
7eeb043
to
4e1abcd
Compare
a169516
to
7bf4d36
Compare
4e1abcd
to
4f7e4fc
Compare
7bf4d36
to
b348133
Compare
4f7e4fc
to
b1248c5
Compare
b1248c5
to
5c17922
Compare
b348133
to
2284f6a
Compare
5c17922
to
ce6238a
Compare
2284f6a
to
54c5d23
Compare
ce6238a
to
4353901
Compare
54c5d23
to
f0e65e8
Compare
4353901
to
ee8b49d
Compare
f0e65e8
to
165c95d
Compare
ee8b49d
to
71a0984
Compare
71a0984
to
6b85dd2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #807 +/- ##
==========================================
+ Coverage 51.38% 51.48% +0.10%
==========================================
Files 30 31 +1
Lines 4231 4296 +65
==========================================
+ Hits 2174 2212 +38
- Misses 1870 1895 +25
- Partials 187 189 +2 ☔ View full report in Codecov by Sentry. |
6b85dd2
to
c19278f
Compare
@@ -612,6 +634,48 @@ func (s streamReader[T]) Result() (result, error) { | |||
return res, fmt.Errorf("didn't receive a result") | |||
} | |||
|
|||
// setStatusFromGRPCErr sets the Update object status blocks based on the result of a gRPC error. |
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.
This is the core logic for determining if we have a structured error, and uses it for surfacing information about Pulumi errors back to the user.
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.
Looking really good
fix: also set the .Status.Message field when updating Update CR status
c19278f
to
a3788c0
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.
LGTM
Proposed changes
Surfaces Locked Stack Errors:
409
(Conflict) error.Improves Stack CR Status Updates:
UpdateCR.status.message
is now correctly propagated toStackCR.status.lastUpdate.message
.Example Stack CR Status Block
Example Update CR Status Block
Testing
Related Issues
Fixes: #806
Fixes: #736