Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #104 +/- ##
=======================================
Coverage 99.71% 99.71%
=======================================
Files 156 158 +2
Lines 3163 3187 +24
Branches 745 757 +12
=======================================
+ Hits 3154 3178 +24
Misses 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| projectRoot: path.join(__dirname, '../../..') | ||
| }) | ||
|
|
||
| const metadataCorrectionServiceRole = new iam.Role(this, 'MetadataCorrectionServiceRole', { |
There was a problem hiding this comment.
Would we just reuse the existing one const listenerRole = new iam.Role(this, 'CmrKeywordEventsProcessorRole', { assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'), managedPolicies: [ iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSLambdaBasicExecutionRole') ] })
There was a problem hiding this comment.
I changed this to let CDK create the execution role for each Lambda automatically instead of defining explicit roles here.
The previous roles were both just standard Lambda execution roles with AWSLambdaBasicExecutionRole, and the real permissions are granted later through the queue/topic grants. Letting CDK create the roles keeps the stack smaller, avoids boilerplate, and still gives each Lambda its own role so permissions stay scoped independently.
| role: metadataCorrectionServiceRole, | ||
| depsLockFilePath: path.join(__dirname, '../../../package-lock.json'), | ||
| projectRoot: path.join(__dirname, '../../..') |
There was a problem hiding this comment.
I see the other lambdas on kmsStack have this I am wondering if this is actually needed now though. Side thing it would be a nice to have to move the runtime to a single place so that when we go to node_24 it can just go all at once. We wouldn't have any use-case where some node lambdas would be in 22 vs XX
There was a problem hiding this comment.
I went ahead and centralized the Node Lambda runtime so we only define it in one place now.
I added a shared NODE_LAMBDA_RUNTIME constant and updated both this stack and the existing KMS Lambda helper to use it. That keeps the runtime consistent across the CDK app and makes the future Node runtime bump a single change instead of a sweep through multiple stacks/helpers.
| /** | ||
| * Metadata correction service placeholder that consumes metadata correction requests from SQS. | ||
| * | ||
| * This proves the SNS/SQS/Lambda plumbing for KMS-676. KMS-675B will replace the stubbed |
There was a problem hiding this comment.
| * This proves the SNS/SQS/Lambda plumbing for KMS-676. KMS-675B will replace the stubbed |
There was a problem hiding this comment.
Yeah changed to a follow-on ticket for now. We'll sort out the tickets needed next week.
Overview
What is the feature?
KMS-676 adds the metadata correction request messaging plumbing that will let KMS publish collection metadata correction requests for downstream processing.
This creates the SNS/SQS/Lambda harness for metadata correction requests. The actual metadata fetch/update logic is intentionally left as follow-up work.
What is the Solution?
This branch adds a FIFO SNS topic and FIFO SQS queue for metadata correction requests in
CmrEventProcessingStack.Key changes:
kms-<stage>-metadata-correction-requests.fifoSNS topic in CDK.metadataCorrectionService, a placeholder Lambda consumer for the metadata correction queue.cmrKeywordEventsListenerto publish a metadata correction request after receiving a keyword event.collectionConceptIdas the FIFOMessageGroupIdso correction requests for the same collection are serialized.Follow-up TODOs are documented in code for:
What areas of the application does this impact?
CmrEventProcessingStackTesting
curl -i -X POST 'http://127.0.0.1:3013/publish?name=v1.0.0'Local verification performed with keyword:
Observed metadata correction payload:
{ "source": "cmrKeywordEventsListener", "collectionConceptId": "C0000000000-KMS", "keywordEvent": { "eventType": "INSERTED", "scheme": "sciencekeywords", "uuid": "7df7859e-46a1-48ef-97fd-413c630a9a7b", "newKeywordPath": "LOCAL TEST KEYWORD 1776817644" } }Attachments
Please include relevant screenshots or files that would be helpful in reviewing and verifying this change.
Checklist