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

chore(maintenance): switch to unmanaged log group for functions #3014

Merged
merged 6 commits into from
Sep 10, 2024

Conversation

dreamorosi
Copy link
Contributor

Summary

Changes

Please provide a summary of what's being changed

This PR updates the base TestNodejsFunction CDK construct so that it now creates and manages its own Amazon CloudWatch LogGroup resource.

With this change we can now set the retention policy directly on the group itself which removes the need of using the logRetention property on the NodejsFunction construct. A nice side effect of this removal is that CDK no longer has to create a custom resource to update the log retention after the fact, thus reducing the number of resources deployed for each test.

Please add the issue number below, if no issue is present the PR might get blocked and not be reviewed

Issue number: closes #3013


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@dreamorosi dreamorosi self-assigned this Sep 2, 2024
@dreamorosi dreamorosi requested review from a team as code owners September 2, 2024 17:08
@pull-request-size pull-request-size bot added the size/S PR between 10-29 LOC label Sep 2, 2024
@dreamorosi dreamorosi force-pushed the chore/e2e_fn_loggroup branch from c9d9013 to 3259b95 Compare September 2, 2024 17:17
@dreamorosi
Copy link
Contributor Author

Blocked by #3012

@dreamorosi dreamorosi added blocked This item's progress is blocked by external dependency or reason do-not-merge This item should not be merged labels Sep 2, 2024
@pull-request-size pull-request-size bot added size/M PR between 30-99 LOC and removed size/S PR between 10-29 LOC labels Sep 5, 2024
@boring-cyborg boring-cyborg bot added the tests PRs that add or change tests label Sep 5, 2024
@pull-request-size pull-request-size bot added size/S PR between 10-29 LOC and removed size/M PR between 30-99 LOC labels Sep 10, 2024
Copy link

@dreamorosi
Copy link
Contributor Author

@dreamorosi dreamorosi removed the do-not-merge This item should not be merged label Sep 10, 2024
@dreamorosi dreamorosi merged commit fbfd0af into main Sep 10, 2024
37 of 39 checks passed
@dreamorosi dreamorosi deleted the chore/e2e_fn_loggroup branch September 10, 2024 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This item's progress is blocked by external dependency or reason size/S PR between 10-29 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maintenance: create own LogGroup resource in integration tests
2 participants