-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
fix(AWS Deploy): Correctly identify "no updates" error during deploy #9248
fix(AWS Deploy): Correctly identify "no updates" error during deploy #9248
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9248 +/- ##
==========================================
- Coverage 86.96% 86.94% -0.02%
==========================================
Files 315 315
Lines 11707 11721 +14
==========================================
+ Hits 10181 10191 +10
- Misses 1526 1530 +4
Continue to review full report at Codecov.
|
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.
@pgrzesik Looks good, I've just proposed one improvement, that should clean up our logic
lib/plugins/aws/lib/updateStack.js
Outdated
const cfData = await this.provider.request('CloudFormation', 'updateStack', params); | ||
await this.monitorStack('update', cfData); | ||
} catch (e) { | ||
if (!e.message.includes(NO_UPDATE_MESSAGE)) { |
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.
I think this error may happen only on CloudFormation.updateStack
call right?
In such case, I'd wrap with try/catch only provider.request
invocation.
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.
good catch - that's correct 👍
71ac694
to
440deb7
Compare
440deb7
to
5e55e29
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.
Looks great 👍
Closes: #9241