chore(cli): env delete should not proceed when it is used in any pipelines #5246
Conversation
|
🍕 Here are the new binary sizes!
|
Codecov Report
@@ Coverage Diff @@
## mainline #5246 +/- ##
============================================
- Coverage 69.79% 69.78% -0.01%
============================================
Files 296 296
Lines 44513 44544 +31
Branches 286 286
============================================
+ Hits 31068 31087 +19
- Misses 11942 11952 +10
- Partials 1503 1505 +2
|
| return fmt.Errorf("get pipeline %s: %w", pipeline.ResourceName, err) | ||
| } | ||
| for _, stage := range info.Stages { | ||
| if strings.TrimPrefix(stage.Name, deploy.StageFullNamePrefix) == o.name { |
There was a problem hiding this comment.
Is this the most durable way we can find this out? Do we stick the manifest in the pipeline metadata?
There was a problem hiding this comment.
That's a very good question. We've never changed our naming pattern for this and I am making it more durable by extracting deploy.StageFullNamePrefix.
Do we stick the manifest in the pipeline metadata?
Unfortunately we don't do that for manifest :(
There was a problem hiding this comment.
Unfortunately we don't do that for manifest :(
Maybe we should! (later)
| getter.EXPECT().GetPipeline("mockResourceName").Return(&codepipeline.Pipeline{ | ||
| Stages: []*codepipeline.Stage{ | ||
| { | ||
| Name: "DeployTo-test", |
There was a problem hiding this comment.
This test having the name hardcoded helps with the durability too because it should catch any breaking name change 👍
| @@ -188,6 +188,10 @@ func (o *deleteAppOpts) Ask() error { | |||
| // It removes all the services from each environment, the environments, the pipeline S3 buckets, | |||
| // the pipeline, the application, removes the variables from the config store, and deletes the local workspace. | |||
| func (o *deleteAppOpts) Execute() error { | |||
| if err := o.deletePipelines(); err != nil { | |||
There was a problem hiding this comment.
maybe a comment why it needs to go first? or just in the Execute() description
There was a problem hiding this comment.
Updated the function comment. I think it's pretty obvious. Deleting the pipelines in the middle (previous impl) should be more confusing because nothing depends on a pipeline.
CaptainCarpensir
added
the
do-not-merge
iamhopaul123
removed
the
do-not-merge
…lines (aws#5246) By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.