move more lambdas over to Sr lambda construct #3107
Conversation
| new StripeDisputes(app, 'stripe-disputes-CODE', { | ||
| stack: 'support', | ||
| stage: 'CODE', | ||
| domainName: `stripe-disputes-code.${supportApisDomain}`, | ||
| hostedZoneId: supportHostedZoneId, | ||
| certificateId: supportCertificateId, | ||
| }); | ||
| new StripeDisputes(app, 'stripe-disputes-PROD', { | ||
| stack: 'support', | ||
| stage: 'PROD', | ||
| domainName: `stripe-disputes.${supportApisDomain}`, | ||
| hostedZoneId: supportHostedZoneId, | ||
| certificateId: supportCertificateId, | ||
| }); |
There was a problem hiding this comment.
it's better not to repeart the parameters every time here and in the tests, as they're more likely to get mixed up, now the stacks are constructed with a standard incantation above
| new MetricPushApi(app, 'CODE', 'membership-CODE-metric-push-api'); | ||
| new MetricPushApi(app, 'PROD', 'membership-PROD-metric-push-api'); |
There was a problem hiding this comment.
not sure why this needs a specific stack name, but I kept it just in case
| new MParticleApi(app, 'mparticle-api-PROD', { | ||
| stack: 'support', | ||
| stage: 'PROD', | ||
| }); |
There was a problem hiding this comment.
don't forget to load the "large diff"s below and review them all
(I will comment on the first instance of each interesting thing)
| { | ||
| "Key": "App", | ||
| "Value": "alarms-handler", | ||
| }, |
There was a problem hiding this comment.
using SrStack ensures that everything is tagged correctly with the App, (this should happen anyway via riffraff/cfn, but GuCDK makes it explicit if it knows the app)
| { | ||
| "Key": "Stack", | ||
| "Value": "membership", | ||
| "Value": "support", |
There was a problem hiding this comment.
as with most of the tests, for some reason it was passing in membership. Now it's not passed in at all so it is definitely consistent.
johnduffell
force-pushed
the
jd/convert-more-stacks
branch
2 times, most recently
from
3cded12 to
73d2341
Compare
| }, | ||
| "MetricPushDNSRecord": { | ||
| "Properties": { | ||
| "Comment": "CNAME for metric-push-api API CODE", |
There was a problem hiding this comment.
I don't think this comment goes anywhere, I was looking in AWS console and I couldn't see it
johnduffell
force-pushed
the
jd/convert-more-stacks
branch
from
73d2341 to
5f8653a
Compare
| "Comment": "CNAME for metric-push-api API CODE", | ||
| "HostedZoneName": "support.guardianapis.com.", | ||
| "Name": "metric-push-api-code.support.guardianapis.com", | ||
| "HostedZoneId": "Z1E4V12LQGXFEC", |
There was a problem hiding this comment.
some places use HostedZoneName and some HostedZoneId, better to be consistent
| "Ref": "AWS::AccountId", | ||
| }, | ||
| ":certificate/b384a6a0-2f54-4874-b99b-96eeff96c009", | ||
| ":certificate/c1efc564-9ff8-4a03-be48-d1990a3d79d2", |
There was a problem hiding this comment.
this was actually wrong in the snapshot
supportCertificateId instead of membershipCertificateId
| "EvaluationPeriods": 1, | ||
| "MetricName": "5XXError", | ||
| "Namespace": "AWS/ApiGateway", | ||
| "Period": 60, |
There was a problem hiding this comment.
metric push API is a bit broken in that it was in membership stack but uses the support domain. I think it's best to make sure it's on support all throughout.
johnduffell
force-pushed
the
jd/convert-more-stacks
branch
2 times, most recently
from
dc49d0e to
8e9c8f5
Compare
johnduffell
force-pushed
the
jd/convert-more-stacks
branch
from
8e9c8f5 to
9c5f5e8
Compare
# Conflicts: # cdk/lib/__snapshots__/stripe-disputes.test.ts.snap # cdk/lib/stripe-disputes.test.ts # cdk/lib/stripe-disputes.ts
Reettaphant
added
maintenance
|
closing as it was too big so I extracted plenty of smaller PRs from this one, whatever is left over is conflicting and unusable |
Following on from recent work to add a reusable SR domain construct #3106
and a reusable SR lambda construct
It appears that using them can reduce duplication and make our stacks more readable and consistent.
It also means that changes e.g. adding en environment variable to tell node to use source maps, would be a one liner.
This PR moves a bunch more stacks over to use the above constructs.