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

fix(cloudformation): should not throw ErrChangeSetEmpty when CFN tmp has bad reference #2270

Merged
merged 4 commits into from
May 7, 2021

Conversation

iamhopaul123
Copy link
Contributor

fixes #2089, fixes #2257

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

@iamhopaul123 iamhopaul123 requested a review from a team as a code owner May 5, 2021 22:20
@iamhopaul123 iamhopaul123 requested a review from bvtujo May 5, 2021 22:20
@iamhopaul123
Copy link
Contributor Author

iamhopaul123 commented May 5, 2021

Before the fix:

✘ execute svc deploy: deploy service: change set with name copilot-197f5bc9-0499-40c4-8ed0-bb5c69b66b6a for stack demo-test-hello has no changes

After the fix:

✘ Proposing infrastructure changes for stack demo-test-hello
✘ deploy service: wait for creation of change set copilot-e6e40e0a-7b2e-45bc-94df-50e75c027930 for stack demo-test-hello: ResourceNotReady: failed waiting for successful resource state: Template format error: Unresolved resource dependencies [mys3s] in the Resources block of the template

@bvtujo
Copy link
Contributor

bvtujo commented May 6, 2021

omg if this is what I think it is... ❤️ ❤️ ❤️ ❤️ ❤️ ❤️

"github.com/stretchr/testify/require"
)

func TestChangeSet_createAndExecute(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we instead capture this part of our exposed APIs like the UpdateStack test instead of creating a separate one here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed~

@efekarakus efekarakus added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label May 6, 2021
@iamhopaul123 iamhopaul123 force-pushed the throw-correct-change-set-error branch from 9bb5db0 to 13a320f Compare May 6, 2021 19:36
@iamhopaul123 iamhopaul123 removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label May 6, 2021
Copy link
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

This is so good, PH. The new tests look amazing

@mergify mergify bot merged commit 9e80236 into aws:mainline May 7, 2021
thrau pushed a commit to localstack/copilot-cli-local that referenced this pull request Dec 9, 2022
…has bad reference (aws#2270)

fixes aws#2089, fixes aws#2257

<!-- Provide summary of changes -->

<!-- Issue number, if available. E.g. "Fixes aws#31", "Addresses aws#42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants