Skip to content
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

Jk/cumulus 3940 to main from #3877 (#3877) #3890

Merged
merged 5 commits into from
Jan 3, 2025

Conversation

Jkovarik
Copy link
Member

@Jkovarik Jkovarik commented Dec 20, 2024

Summary: Summary of changes

Addresses CUMULUS-3940

Changes

This PR brings forward the merged code from #3877 on the 18.5.x release series.

PR Checklist

  • Update CHANGELOG
  • Unit tests
  • Ad-hoc testing - Deploy changes and test manually
  • Integration tests

* Update message recovery/granule write logic to properly use esClient

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

* Update Core to allow DLA recovery configuration

This commit updates:

- archive/cumulus/example to pass through memory
configuration options to the fargate task definition

* Add api performance test

* Update docs/changelog

* Update CHANGELOG and documentation

* Update CHANGELOG

* Fix linting

* Fix units

* Update dead letter archive feature doc

* Update test spec

* Update logging, make perf test script executable

* Fix broken package.json ava exclusion configuration

* Add zod parsing to dead letter endpoint

* Update tf-modules/archive/async_operation.tf

Co-authored-by: jennyhliu <[email protected]>

* Update tf-modules/archive/async_operation.tf

Co-authored-by: jennyhliu <[email protected]>

* Address db pool configuration concern in PR

* Update env config passthroughs/make log/docs consistent

* Update tf-modules/archive/async_operation.tf

Co-authored-by: jennyhliu <[email protected]>

* Update tf-modules/archive/async_operation.tf

Co-authored-by: jennyhliu <[email protected]>

* Update per PR suggestion

* Update concurrency defaults for consistency

* Update startAsyncOperations to allow for optional container names

* Update dead letter archive endpoint to specify new container name

* Update API defaults/units to 30 to match system defaults

* Fix defaults for endpoint tests

* Add changed params to demonstrate payload handling

* Updarte coverage metric

Updated code in this module doesn't significantly impact test
coverage, other than increasing the denominator.

* fixup

* Update performance tests to match documented defaults

---------

Co-authored-by: jennyhliu <[email protected]>
Copy link
Contributor

@Nnaga1 Nnaga1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had some comments about some formatting stuff but looks good! I'll deploy + test + inspect and approve when thats good 🙌

docs/features/dead_letter_archive.md Show resolved Hide resolved
example/cumulus-tf/variables.tf Show resolved Hide resolved
packages/api/lib/writeRecords/write-granules.js Outdated Show resolved Hide resolved
tf-modules/archive/variables.tf Show resolved Hide resolved
tf-modules/cumulus/variables.tf Show resolved Hide resolved
@Jkovarik Jkovarik changed the title Jk/cumulus 3940 18.5.x (#3877) Jk/cumulus 3940 to main from #3877 (#3877) Jan 3, 2025
@Jkovarik Jkovarik removed the in review label Jan 3, 2025
@Jkovarik Jkovarik requested a review from Nnaga1 January 3, 2025 17:21
Copy link
Contributor

@Nnaga1 Nnaga1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deployed locally and it went fine 🙌 also verified in AWS, great work as usual!! 😄 🎆

@Jkovarik Jkovarik merged commit 440e084 into master Jan 3, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants