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

feat(cli): put s3 bucket lifecycle policy for aws tests #2434

Conversation

bernardobridge
Copy link
Contributor

@bernardobridge bernardobridge commented Jan 19, 2024

Description

  • Sets s3 lifecycle policy for Fargate (e314fbe) and Lambda tests (48c76f9)
  • A small refactor (46fd000) was done for Lambda to reuse utilities that were already there, and seemed to already account for Lambda region nuance. This way it automatically uses the created setBucketLifecyclePolicy function

Note: as usual, best to review this PR commit by commit (using the commits I listed above).

Please review the following:

  • Actual lifecycle rules chosen. I chose 2 days for things containing test data, to give some time for people to be able to review errors, as sometimes you might only get to it next day, and 7 days for things that are mostly metadata (small objects anyway)
  • Should we have a configurable way to override these? Even if it's an environment variable override to start with? People might have compliance requirements and such that require them to keep it longer.
  • I made the setBucketLifecyclePolicy not error when it fails, as it's not a crucial thing to be done (and can always be done on future runs). But perhaps it's better to actually error?

Pre-merge checklist

  • Does this require an update to the docs? Yes
  • Does this require a changelog entry? Yes

@bernardobridge bernardobridge requested a review from a team January 19, 2024 14:00
@bernardobridge bernardobridge merged commit 5b07d5e into main Feb 5, 2024
17 of 18 checks passed
@bernardobridge bernardobridge deleted the bernardobridge/oss-17-put-s3-bucket-lifecycle-policy-for-aws-tests branch February 5, 2024 14:24
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.

None yet

1 participant