Skip to content
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

Reorder cleanup step by put subnet and vpc to the last to avoid conflict #2776

Merged
merged 1 commit into from
May 10, 2023

Conversation

oilbeater
Copy link
Collaborator

What type of this PR

Examples of user facing changes:

  • Bug fixes

Which issue(s) this PR fixes:

Fixes #(issue-number)

WHAT

copilot:summary

copilot:poem

HOW

copilot:walkthrough

@oilbeater oilbeater added bug Something isn't working vpc labels May 9, 2023
@oilbeater oilbeater requested a review from zbb88888 May 9, 2023 08:54
@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2023

  • The cleanup script should be refactored to group resources by type and delete them in a more efficient way. For example, all subnets could be deleted at once instead of looping through each one individually.
  • There is a potential bug in the script where the "set +e" command is not followed by a corresponding "set -e" command, which could cause errors to be ignored and not properly handled.
  • It would be helpful to add comments to the script to explain what each section is doing and why certain resources are being deleted in a specific order.
  • The script could benefit from error handling and logging to better track any issues that may arise during the cleanup process.
  • It may be useful to add a confirmation prompt before deleting resources to prevent accidental deletions.

@oilbeater oilbeater merged commit b1a339b into master May 10, 2023
@oilbeater oilbeater deleted the fix/cleanup branch May 10, 2023 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working vpc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants