-
Notifications
You must be signed in to change notification settings - Fork 154
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
WIP: fix(ecsservice): triggers required when using forceNewDeployment #4262
Conversation
PR is now waiting for a maintainer to run the acceptance tests. |
/run-acceptance-tests |
Fixes #4162 |
Hi @stooj sorry dropped the ball here. @flostadler could you have a look as part of ops, something's not quite setup in CI to accept docs contributions but if we rerun |
No worries - I didn't spend the time to set up my local environment for dev contributions (I thought "it's just the docs") but since it's auto-generated I should have checked it locally. |
You're pointing out a real miss we have here, we'd love to remove friction from "just the docs" contributions. I'll talk to the team. |
@stooj if you want I can run the re-gen steps. Just lmk |
@flostadler That'd be great, thanks. |
PR is now waiting for a maintainer to run the acceptance tests. |
@@ -245414,7 +245414,7 @@ | |||
}, | |||
"forceNewDeployment": { | |||
"type": "boolean", | |||
"description": "Enable to force a new task deployment of the service. This can be used to update tasks to use a newer Docker image with same image/tag combination (e.g., `myimage:latest`), roll Fargate tasks onto a newer platform version, or immediately deploy `ordered_placement_strategy` and `placement_constraints` updates.\n" | |||
"description": "Enable to force a new task deployment of the service. This can be used to update tasks to use a newer Docker image with same image/tag combination (e.g., `myimage:latest`), roll Fargate tasks onto a newer platform version, or immediately deploy `ordered_placement_strategy` and `placement_constraints` updates.\nWhen using the forceNewDeployment property you also need to configure the Triggers property.\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we have two different styles of property naming in the description now. (placement_constraints
vs forceNewDeployment
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my understanding forceNewDeployment is the preferred style when we cannot inflect toward the target language.
@stooj Done! |
PR is now waiting for a maintainer to run the acceptance tests. |
provider/doc_edits.go
Outdated
"This can be used to update tasks to use a newer Docker image with same image/tag combination "+ | ||
"(e.g., `myimage:latest`), roll Fargate tasks onto a newer platform version, or immediately deploy "+ | ||
"`ordered_placement_strategy` and `placement_constraints` updates.\n"+ | ||
"When using the forceNewDeployment property you also need to configure the Triggers property.\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Triggers capitalized intentionally? IN the most used language TypeScript it's probably not capitalized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
/run-acceptance-tests |
Please view the PR build: https://github.com/pulumi/pulumi-aws/actions/runs/10473376111 |
PR is now waiting for a maintainer to run the acceptance tests. |
/run-acceptance-tests |
Please view the PR build: https://github.com/pulumi/pulumi-aws/actions/runs/10494287999 |
Acceptance tests were successful. Bypassing branch protection rules because they're waiting for sentinel |
This PR has been shipped in release v6.50.1. |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@pulumi/aws](https://pulumi.io) ([source](https://togithub.com/pulumi/pulumi-aws)) | dependencies | patch | [`6.50.0` -> `6.50.1`](https://renovatebot.com/diffs/npm/@pulumi%2faws/6.50.0/6.50.1) | --- ### Release Notes <details> <summary>pulumi/pulumi-aws (@​pulumi/aws)</summary> ### [`v6.50.1`](https://togithub.com/pulumi/pulumi-aws/releases/tag/v6.50.1) [Compare Source](https://togithub.com/pulumi/pulumi-aws/compare/v6.50.0...v6.50.1) ##### Does the PR have any schema changes? Found 1 breaking change: ##### Resources - `🟢` "aws:pinpoint/gcmChannel:GcmChannel": required: "apiKey" property is no longer Required ##### New functions: - `route53/getZones.getZones` - `ssoadmin/getPermissionSets.getPermissionSets` ##### What's Changed - WIP: fix(ecsservice): triggers required when using forceNewDeployment by [@​stooj](https://togithub.com/stooj) in [pulumi/pulumi-aws#4262 - Upgrade upstream to v5.63.1 by [@​flostadler](https://togithub.com/flostadler) in [pulumi/pulumi-aws#4390 ##### New Contributors - [@​stooj](https://togithub.com/stooj) made their first contribution in [pulumi/pulumi-aws#4262 **Full Changelog**: pulumi/pulumi-aws@v6.50.0...v6.50.1 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41MS4xIiwidXBkYXRlZEluVmVyIjoiMzguNTEuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsidHlwZS9wYXRjaCJdfQ==--> Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@pulumi/aws](https://pulumi.io) ([source](https://togithub.com/pulumi/pulumi-aws)) | dependencies | patch | [`6.50.0` -> `6.50.1`](https://renovatebot.com/diffs/npm/@pulumi%2faws/6.50.0/6.50.1) | --- ### Release Notes <details> <summary>pulumi/pulumi-aws (@​pulumi/aws)</summary> ### [`v6.50.1`](https://togithub.com/pulumi/pulumi-aws/releases/tag/v6.50.1) [Compare Source](https://togithub.com/pulumi/pulumi-aws/compare/v6.50.0...v6.50.1) ##### Does the PR have any schema changes? Found 1 breaking change: ##### Resources - `🟢` "aws:pinpoint/gcmChannel:GcmChannel": required: "apiKey" property is no longer Required ##### New functions: - `route53/getZones.getZones` - `ssoadmin/getPermissionSets.getPermissionSets` ##### What's Changed - WIP: fix(ecsservice): triggers required when using forceNewDeployment by [@​stooj](https://togithub.com/stooj) in [pulumi/pulumi-aws#4262 - Upgrade upstream to v5.63.1 by [@​flostadler](https://togithub.com/flostadler) in [pulumi/pulumi-aws#4390 ##### New Contributors - [@​stooj](https://togithub.com/stooj) made their first contribution in [pulumi/pulumi-aws#4262 **Full Changelog**: pulumi/pulumi-aws@v6.50.0...v6.50.1 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41MS4xIiwidXBkYXRlZEluVmVyIjoiMzguNTEuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsidHlwZS9wYXRjaCJdfQ==--> Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
There is a note saying you need to set
forceNewDeployment
when addingtriggers
.The reverse is also true, so adding a note to say that
triggers
are required when addingforceNewDeployment
.