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

[Resolve #632] Prevent before_update on no-ops #661

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

devenney
Copy link
Contributor

  • Create change sets before any update operation to test for changes
  • Only execute before_update hook when change set is empty
  • Update tests to mock non-empty change sets

Resolves #632.

Co-authored-by: Amy Robertson [email protected]
Co-authored-by: Chip Wolf [email protected]
Co-authored-by: David Munn [email protected]


Signed-off-by: Brendan Devenney [email protected]

Resolves the issue of executing before_update before checking if there are updates. When there are no updates to perform, Sceptre exits without executing after_update. The findings in #632 suggest before_update's behaviour is unintended rather than after_update.

If you feel it's worth adding unit tests to test that the hooks are/aren't run given the relevant Stack states, let me know. Presumably that would constitute a larger piece of work as it touches all hooks (create, update, delete).

PR Checklist

  • Wrote a good commit message & description [see guide below].
  • Commit message starts with [Resolve #issue-number].
  • Added/Updated unit tests.
  • Added/Updated integration tests (if applicable).
  • All unit tests (make test) are passing.
  • Used the same coding conventions as the rest of the project.
  • The new code passes flake8 (make lint) checks.
  • The PR relates to only one subject with a clear title.
    and description in grammatically correct, complete sentences.

Approver/Reviewer Checklist

  • Before merge squash related commits.

Other Information

Guide to writing a good commit

* Create change sets before any update operation to test for changes
* Only execute before_update hook when change set is empty
* Update tests to mock non-empty change sets

Resolves Sceptre#632.

Co-authored-by: Amy Robertson <[email protected]>
Co-authored-by: Chip Wolf <[email protected]>
Co-authored-by: David Munn <[email protected]>

Signed-off-by: Brendan Devenney <[email protected]>
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.

before_update hook always runs
1 participant