Fix intermittent IAM permission failures in delete-deployment command#958
Fix intermittent IAM permission failures in delete-deployment command#958AlexDaines wants to merge 1 commit intomainfrom
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #958 +/- ##
===========================================
- Coverage 62.14% 35.11% -27.03%
===========================================
Files 294 294
Lines 10796 10796
Branches 1616 1616
===========================================
- Hits 6709 3791 -2918
- Misses 3487 6659 +3172
+ Partials 600 346 -254 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| private readonly IAWSUtilities _awsUtilities; | ||
| private readonly IProjectParserUtility _projectParserUtility; | ||
| private readonly IAWSResourceQueryer _awsResourceQueryer; | ||
| private IAmazonCloudFormation? _cloudFormationClient; |
There was a problem hiding this comment.
why does this need to be optional now?
There was a problem hiding this comment.
when the constructor runs the object is created as null and stays null until we do the assignment in ExecuteAsync. So without the ?, C# would want us to initialize it in the constructor, which is the bug we're fixing
There was a problem hiding this comment.
can you keep the code as is but then just add the extra line you added on line 83? so the only change will be recreating the cloudformation client. im not a huge fan of the optional values
|
You should be targeting |
| private readonly IAWSUtilities _awsUtilities; | ||
| private readonly IProjectParserUtility _projectParserUtility; | ||
| private readonly IAWSResourceQueryer _awsResourceQueryer; | ||
| private IAmazonCloudFormation? _cloudFormationClient; |
There was a problem hiding this comment.
can you keep the code as is but then just add the extra line you added on line 83? so the only change will be recreating the cloudformation client. im not a huge fan of the optional values
|
Do you mind if I take a stab at it? I want to move some things around. |
Please do! No worries. |
3 participants
Issue #, if available:
Package Verifier Canary failed during SmokeTests execution (V1823703386)
Package Verifier Canary failed during SmokeTests execution (V1823700902)
Description of changes:
Fixed CloudFormation client initialization timing issue in DeleteDeploymentCommand
Client was being created with wrong AWS credentials during constructor execution
Moved client creation to ExecuteAsync after credentials are properly configured
Testing
dotnet buildBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.