[terraform plugin] Remove stage config for TERRAFORM_APPLY#6259
Conversation
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6259 +/- ##
==========================================
+ Coverage 28.69% 28.71% +0.01%
==========================================
Files 560 560
Lines 59738 59749 +11
==========================================
+ Hits 17144 17159 +15
+ Misses 41278 41271 -7
- Partials 1316 1319 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return sdk.StageStatusFailure | ||
| } | ||
| // TODO: use stageConfig if this stage has any options in the future. | ||
| // stageConfig := config.TerraformApplyStageOptions{} |
There was a problem hiding this comment.
@ffjlabo Should we remove the config.TerraformApplyStageOptions structure and this commented code block? If it's not being used, better to remove it 🤔
There was a problem hiding this comment.
Also, the root cause of this issue is more like the input.Request.StageConfig is nil, so it's better to have a nil check for input.Request.StageConfig before unmarshalling, wdyt?
There was a problem hiding this comment.
@khanhtc1202
As you said, this option is not used for now. I will remove this logic and struct.
pipecd/pkg/config/application_terraform.go
Lines 60 to 73 in af58b81
Also, the root cause of this issue is more like the input.Request.StageConfig is nil, so it's better to have a nil check for input.Request.StageConfig before unmarshalling, wdyt?
It is good for the work around. But we might solve this problem in the piped or SDK.
But I will remove this logic.
There was a problem hiding this comment.
Thanks, please remove the code block instead of comment out.
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
12863f8 to
cf89000
Compare
khanhtc1202
left a comment
There was a problem hiding this comment.
Thank you! 🙏
Note: We will make another issue for updating the SDK to ensure the stage config is not nil.
What this PR does:
as title
Why we need it:
Currently, the
TERRAFORM_APPLYstage doesn't have any options.pipecd/pkg/app/pipedv1/plugin/terraform/config/config.go
Lines 61 to 63 in 96c15d8
So we don't need the logic for now. Also, it led the failure to execute
TERRAFORM_APPLYas QuickSyncWhich issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: