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

updates to the dynamic reference lab templates \ module #195

Merged
merged 3 commits into from
Apr 3, 2023

Conversation

dkovvuri
Copy link
Member

@dkovvuri dkovvuri commented Apr 1, 2023

What does this PR do and why?

Describe in detail what your merge request does and why.

Matteo brought to my attention about an issue with the lab templates re: DeletionPolicy. By default, the DeletionPolicy attribute uses Delete . However, there is an exception (see Note Exception in the page) for RDS. As a result, when running the database.yaml template in the lab above (this template is present in both the workspace and solutions relevant subdirectories), a snapshot on delete will be created and persisted in the account and region.

This fix explicitly sets the DeletionPolicy to Delete attached to the RDS resource and explains the policy choice in the module.

PR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

  • [✓] Added/Updated documentation if applicable.
  • [✓] Pre-commit checks passed.
  • [✓] Lint and Nag checks passed.
  • [✓] If releasing a new version, have you bumped the version make version part=<major|minor|patch>?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dkovvuri dkovvuri requested a review from a team as a code owner April 1, 2023 16:20
Copy link
Contributor

@mrinaudo-aws mrinaudo-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @dkovvuri ! Making small changes as well.

content/intermediate/templates/dynamic-references/index.md Outdated Show resolved Hide resolved
@mrinaudo-aws mrinaudo-aws self-requested a review April 2, 2023 13:00
Copy link
Contributor

@mrinaudo-aws mrinaudo-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rezabekf rezabekf merged commit d3c501d into main Apr 3, 2023
@rezabekf rezabekf deleted the dynamic_ref_updates branch April 3, 2023 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants