-
Notifications
You must be signed in to change notification settings - Fork 6
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
Parameterise cpuArchitecture for GuEcsTask #2486
Conversation
🦋 Changeset detectedLatest commit: 568165a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
689ee93
to
e686804
Compare
@@ -0,0 +1,5 @@ | |||
--- | |||
"@guardian/cdk": major |
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 this really a breaking change? Or is it a minor?
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.
It's breaking because the previous default was to use X86 rather than ARM
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.
Ah! Hadn't spotted that. Are there any scenarios where we'd ever use x86? I wonder if a "paved road" solution would be to not expose cpuArchitecture
at all. Where a service not be be ARM compatible, it'll use an escape hatch to change the architecture.
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.
Sorry I missed this! Sounds good, have updated
76d3665
to
93510f3
Compare
93510f3
to
568165a
Compare
What does this change?
Previously we could only launch x86 ecs tasks. This resolves that.
Note - @akash1810 suggested pulling the ecs task out of GuCDK since it's only used by investigations. V happy to do this but in the short term it would be helpful to have this extra configuration property
How to test
I've tested this by deploying to a project in the investigations account.