Fix terraform plan file should not be deleted #855
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request modify the Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🔇 Additional comments (2)internal/exec/terraform.go (2)Line range hint The cleanup logic for the variable file is correctly maintained, only removing it after the
The modification correctly preserves the plan file when running Let's verify the behavior with different terraform commands: ✅ Verification successfulThe plan file cleanup logic is correctly implemented and consistent with the codebase's behavior The verification confirms that:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify plan file handling across different terraform commands
# Expected: Plan file should be preserved for 'plan' and 'show' commands, but removed for others
# Test 1: Search for plan file cleanup logic
rg -A 5 "PlanFile == \"\"" internal/exec/terraform.go
# Test 2: Search for all terraform show command handling
rg -A 5 "SubCommand.*show" internal/exec/terraform.go
Length of output: 849 Script: #!/bin/bash
# Let's verify the complete context of terraform command handling
# to ensure the plan file cleanup logic is consistent
# Search for all terraform command handling patterns
rg -B 5 -A 10 "info\.SubCommand\s*==" internal/exec/terraform.go
# Check if there are any other places where plan files are handled
rg -A 5 "planFilePath" internal/exec/terraform.go
Length of output: 8159 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
I think you've identified the culprit here causing the problem however I think that our approach is wrong. Rather than by default deleting files for a virtually any command, which is an unexpected behavior, especially for command like running help, I think we should instead identify when if ever we should delete files automatically. We have a separate command called In other words, I think what I'm proposing is we just remove this deletion altogether. |
|
@aknysh what are your thoughts about this? |
we had disabled the deletion altogether once, but then people started complaining that Atmos leaves a lot of files behind :) I agree that the best approach would be to only clean after the commands that actually generate the files All help commands get executed before this part of the code, they don't delete any files |
aknysh
deleted the
feature/dev-2825-atmos-terraform-show-deletes-planfile
branch
|
These changes were released in v1.123.1. |
issue: https://linear.app/cloudposse/issue/DEV-2825/atmos-terraform-show-deletes-planfile
what
why
We should not delete the terraform plan file if the user uses show to check his results.
Test
Steps:
cd examples/quick-start-simple../../build/atmos terraform plan -s dev station../../build/atmos terraform show -s dev station --json dev-station.planfile --skip-initVerified if the file
./examples/quick-start-simple/components/terraform/weather/dev-station.planfilestill exists.Before this change: deleted
After this change: present
references
issue: https://linear.app/cloudposse/issue/DEV-2825/atmos-terraform-show-deletes-planfile
conversation: https://sweetops.slack.com/archives/C031919U8A0/p1733924783112799
Summary by CodeRabbit