CDK Stack for Bulk Export #4273
Conversation
…d S3 and DynamoDB permissions to BulkExportStack
…Lambda for consistency and readability; add DynamoDB and S3 permissions to BulkExportStack
…ortStack and SqsQueues
…ew properties for splitter queue and DLQ
ab295382
requested review from
kr565370 and
patchwork01
and removed request for
patchwork01
...bulk-export-lambda/src/main/java/sleeper/bulkexport/lambda/SqsBulkExportProcessorLambda.java
Outdated
Show resolved
Hide resolved
...xport/bulk-export-lambda/src/main/java/sleeper/bulkexport/lambda/SqsBulkExportProcessor.java
Outdated
Show resolved
Hide resolved
...bulk-export-lambda/src/main/java/sleeper/bulkexport/lambda/SqsBulkExportProcessorLambda.java
Show resolved
Hide resolved
...bulk-export-lambda/src/main/java/sleeper/bulkexport/lambda/SqsBulkExportProcessorLambda.java
Show resolved
Hide resolved
...bulk-export-lambda/src/main/java/sleeper/bulkexport/lambda/SqsBulkExportProcessorLambda.java
Show resolved
Hide resolved
...bulk-export-lambda/src/main/java/sleeper/bulkexport/lambda/SqsBulkExportProcessorLambda.java
Outdated
Show resolved
Hide resolved
java/cdk/src/main/java/sleeper/cdk/stack/bulkexport/BulkExportStack.java
Show resolved
Hide resolved
| * to their own SQS queue can give the role permission to write to their queue | ||
| */ | ||
| IRole role = Objects.requireNonNull(lambda.getRole()); | ||
| instanceProperties.set(CdkDefinedInstanceProperty.BULK_EXPORT_PROCESSOR_LAMBDA_ROLE, role.getRoleName()); |
There was a problem hiding this comment.
I'm not sure this is the role you would need to give permission to write to an output queue. The actual bulk export will be done in a longer running task, rather than in this lambda. Are you expecting the results will come back to the lambda somehow, or will they be published from the long running task?
java/core/src/main/java/sleeper/core/properties/instance/BulkExportProperty.java
Outdated
Show resolved
Hide resolved
java/core/src/main/java/sleeper/core/properties/instance/BulkExportProperty.java
Outdated
Show resolved
Hide resolved
| @@ -254,6 +254,53 @@ public interface CdkDefinedInstanceProperty extends InstanceProperty { | |||
| .description("The ARN of the dead letter queue used when leaf partition querying sleeper.") | |||
| .propertyGroup(InstancePropertyGroup.QUERY) | |||
| .build(); | |||
| // Bulk Export | |||
There was a problem hiding this comment.
These properties are a bit misleading. There's a lambda that's called the bulk export processor lambda, but it's doing the splitting. There's no lambda called the bulk export splitter lambda. Do you want to adjust the names?
There was a problem hiding this comment.
I have added new properties for the splitter. The queues have been renamed to make it clearer
There was a problem hiding this comment.
It looks like these property names are still the same? The ones in BulkExportProperty have changed, but not in CdkDefinedInstanceProperty.
There was a problem hiding this comment.
There are BULK_EXPORT_SPLITTER_* properties as well as BULK_EXPORT_PROCESSOR_*. The latter are to be used in a new lambda (in the next ticket) that will run the actually export.
Make sure you have checked all steps below.
Issue
PR"
Tests
Documentation
separate issue for that below.