Skip to content

fix: env override should be applied in svc package and job deploy#2476

Merged
mergify[bot] merged 2 commits intoaws:mainlinefrom
Lou1415926:fix/env-override
Jun 17, 2021
Merged

fix: env override should be applied in svc package and job deploy#2476
mergify[bot] merged 2 commits intoaws:mainlinefrom
Lou1415926:fix/env-override

Conversation

@Lou1415926
Copy link
Copy Markdown
Contributor

This PR fixes #2474

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

@Lou1415926 Lou1415926 requested a review from a team as a code owner June 17, 2021 16:55
@Lou1415926 Lou1415926 requested review from iamhopaul123 and removed request for a team June 17, 2021 16:55
@Lou1415926 Lou1415926 added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Jun 17, 2021
Copy link
Copy Markdown
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.

Looks good to me; can we add any unit tests or integ tests that will catch changes like this in the future? Maybe if we modify one of the LB web service integ tests that involve env overrides?

@Lou1415926 Lou1415926 removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Jun 17, 2021
@Lou1415926
Copy link
Copy Markdown
Contributor Author

Looks good to me; can we add any unit tests or integ tests that will catch changes like this in the future? Maybe if we modify one of the LB web service integ tests that involve env overrides?

I think we should refactor cli package since svc package, job deploy and svc deploy share some similar processing.

I'm not sure if the current integ test would be a great place to test this. From my understanding the purpose of the stack integ test is to see if the manifest spits out by cli can be transformed into a stack template as expected; however, ApplyEnv happens within cli.

@mergify mergify Bot merged commit c3d1ed2 into aws:mainline Jun 17, 2021
thrau pushed a commit to localstack/copilot-cli-local that referenced this pull request Dec 9, 2022
…s#2476)

This PR fixes aws#2474 

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

Development

Successfully merging this pull request may close these issues.

copilot 1.8: variables and secrets in environments section of manifest not populated from pipeline

4 participants