CUMULUS-3940 -- Forward port to main #3878
Open
+467
−31
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit updates the following: - esClient is properly passed through api lambda/lib methods such that write granules calls from process-s3-dead-letter-archive can pass in an instance of EsClient rather than relying on default per-granule object/client behavior - The API endpoint and related code are updated such that maxDbPool, concurrency and batchSize are exposed as endpoint options, allowing user customization of tuning behavior for the DLA recovery tool - Minor typing/call fixes
This commit updates: - archive/cumulus/example to pass through memory configuration options to the fargate task definition
etcart
reviewed
Dec 11, 2024
| const asyncOperation = await asyncOperations.startAsyncOperation({ | ||
| cluster: process.env.EcsCluster, | ||
| callerLambdaName: getFunctionNameFromRequestContext(req), | ||
| lambdaName: process.env.DeadLetterProcessingLambda, | ||
| asyncOperationTaskDefinition: process.env.AsyncOperationTaskDefinition, | ||
| asyncOperationTaskDefinition: process.env.DeadLetterRecoveryTaskDefinition, |
There was a problem hiding this comment.
so this is our async task definition but using "not the same definition as general async operations" right?
etcart
reviewed
Dec 11, 2024
| path, | ||
| }, | ||
| stackName, | ||
| systemBucket, | ||
| knexConfig: process.env, | ||
| knexConfig: { ...process.env, maxDbPool }, |
There was a problem hiding this comment.
why isn't maxDbPool being gotten originally from the environment? I note that further down in tests it's being configured as an environment variable
etcart
reviewed
Dec 11, 2024
| cumulusMessage, | ||
| collectionCumulusId, | ||
| providerCumulusId, | ||
| knex, | ||
| executionCumulusId, | ||
| esClient, | ||
| }); | ||
|
|
||
| return writeGranulesFromMessage({ |
There was a problem hiding this comment.
are we certain this works? I don't see a change to "writeGranulesFromMessage" that uses an esclient argument, either In the changes here or in the master branch code
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary: Summary of changes
Addresses CUMULUS-3940
Changes
Forward port of #3877 - see discussion in port
PR Checklist