Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

Lambda: refactor tagging#13770

Merged
bentsku merged 2 commits intomainfrom
rgta-lambda
Feb 18, 2026
Merged

Lambda: refactor tagging#13770
bentsku merged 2 commits intomainfrom
rgta-lambda

Conversation

@bentsku
Copy link
Copy Markdown
Contributor

@bentsku bentsku commented Feb 13, 2026 •
edited
Loading

Motivation

This PR refactors Tagging in Lambda to support its integration with RGTA.

The only functional difference is that we are now properly cleaning up the tags when deleting a resource, where before they would stay in the store.

Changes

  • refactor store access and tagging functionality to be even more decoupled (splitting validation from store access)

Tests

Related

@bentsku bentsku added this to the 4.14 milestone Feb 13, 2026
@bentsku bentsku self-assigned this Feb 13, 2026
@bentsku bentsku added aws:lambda AWS Lambda semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes labels Feb 13, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 13, 2026 •
edited
Loading

Test Results - Preflight, Unit

23 123 tests  +12   21 252 ✅ +3   6m 24s ⏱️ -3s
     1 suites ± 0    1 871 💤 +9 
     1 files   ± 0        0 ❌ ±0 

Results for commit 507be02. ± Comparison against base commit 8554f9a.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 13, 2026 •
edited
Loading

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   2m 47s ⏱️ -13s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 507be02. ± Comparison against base commit 8554f9a.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 13, 2026 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 2m 31s ⏱️
3 888 tests 3 639 ✅ 248 💤 1 ❌
3 894 runs  3 639 ✅ 254 💤 1 ❌

For more details on these failures, see this check.

Results for commit 507be02.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 13, 2026 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   1h 32m 54s ⏱️ - 27m 44s
3 864 tests  - 1 351  3 611 ✅  - 1 252  252 💤  - 100  1 ❌ +1 
3 866 runs   - 1 351  3 611 ✅  - 1 252  254 💤  - 100  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 507be02. ± Comparison against base commit 8554f9a.

This pull request removes 1470 and adds 119 tests. Note that renamed tests count towards both. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

tests.aws.services.logs.test_logs_delivery.TestDeliveries ‑ test_create_delivery
tests.aws.services.logs.test_logs_delivery.TestDeliveries ‑ test_delete_delivery
tests.aws.services.logs.test_logs_delivery.TestDeliveries ‑ test_describe_deliveries
tests.aws.services.logs.test_logs_delivery.TestDeliveries ‑ test_get_delivery
tests.aws.services.logs.test_logs_delivery.TestDeliveryDestinationPolicies ‑ test_delete_delivery_destination_policy
tests.aws.services.logs.test_logs_delivery.TestDeliveryDestinationPolicies ‑ test_get_delivery_destination_policy
tests.aws.services.logs.test_logs_delivery.TestDeliveryDestinationPolicies ‑ test_put_delivery_destination_policy
tests.aws.services.logs.test_logs_delivery.TestDeliveryDestinations ‑ test_delete_delivery_destination
tests.aws.services.logs.test_logs_delivery.TestDeliveryDestinations ‑ test_delete_delivery_destination_not_found
tests.aws.services.logs.test_logs_delivery.TestDeliveryDestinations ‑ test_describe_delivery_destinations
…
This pull request removes 117 skipped tests and adds 17 skipped tests. Note that renamed tests count towards both. 
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input4-FAILED]
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_deployed_infra_state
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_populate_data
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_user_clicks_are_stored
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_api_exceptions
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_create_exceptions
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_create_invalid_desiredstate
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_double_create_with_client_token
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_lifecycle
…

tests.aws.services.logs.test_logs_destinations.TestDestinationsTags ‑ test_destination_tags
tests.aws.services.logs.test_logs_export_tasks.TestExportTasks ‑ test_cancel_export_task_not_found
tests.aws.services.logs.test_logs_export_tasks.TestExportTasks ‑ test_create_export_task
tests.aws.services.logs.test_logs_export_tasks.TestExportTasks ‑ test_create_export_task_bucket_not_found
tests.aws.services.logs.test_logs_export_tasks.TestExportTasks ‑ test_create_export_task_log_group_not_found
tests.aws.services.logs.test_logs_export_tasks.TestExportTasks ‑ test_create_export_task_with_prefix
tests.aws.services.logs.test_logs_export_tasks.TestExportTasks ‑ test_describe_export_tasks
tests.aws.services.logs.test_logs_export_tasks.TestExportTasks ‑ test_describe_export_tasks_not_found
tests.aws.services.logs.test_logs_export_tasks.TestExportTasksWithLogs ‑ test_create_export_task_with_logs
tests.aws.services.logs.test_logs_filter_events.TestFilterLogEvents ‑ test_filter_log_events_unknown_token
…

♻️ This comment has been updated with latest results.

@bentsku bentsku marked this pull request as ready for review February 13, 2026 20:33
joe4dev
joe4dev approved these changes Feb 18, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@joe4dev joe4dev left a comment

Choose a reason for hiding this comment

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

Thank you for the clear refactorings 🚀

localstack-core/localstack/services/lambda_/provider.py Outdated
raise ResourceNotFoundException(
"The resource you requested does not exist.", Type="User"
)
# the actual deletion of the ESM is happening synchronously, but we delete the Tags instantly
Copy link
Copy Markdown
Member

@joe4dev joe4dev Feb 18, 2026

Choose a reason for hiding this comment

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

docs nit: I guess this comments intents to say "asynchronously" (instead of synchronously)

❓ Can we clarify in the comment whether that's a convenience behavior (i.e., TODO as intentional parity gap) or matches AWS parity?

Copy link
Copy Markdown
Contributor Author

@bentsku bentsku Feb 18, 2026

Choose a reason for hiding this comment

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

Done, good catch! I've looked at it and it seems the answer is... "it depends" 😅 the behavior I added is similar to get_event_source_mapping, it will start raising ResourceNotFound right after the call to delete_event_source_mapping, but AWS can take some time before starting to raise it.

I've improved the comment to say the tagging deletion behavior is aligned with get_event_source_mapping, and to add more testing around that. Seems like a hard thing to test because it can be almost instant, so not sure we want to capture this kind of behavior (async deletion) in the CRUD layer.

@bentsku
Copy link
Copy Markdown
Contributor Author

bentsku commented Feb 18, 2026

Merging, test failure is unrelated (snapshot failure from lambda env vars): https://github.com/localstack/localstack/runs/63995024560 (test_lambda_init_environment)

@bentsku bentsku merged commit 50501b3 into main Feb 18, 2026
31 of 35 checks passed
@bentsku bentsku deleted the rgta-lambda branch February 18, 2026 12:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@dominikschubert dominikschubert Awaiting requested review from dominikschubert

@dfangl dfangl Awaiting requested review from dfangl

@gregfurman gregfurman Awaiting requested review from gregfurman

1 more reviewer

@joe4dev joe4dev joe4dev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@bentsku bentsku

Labels

aws:lambda AWS Lambda docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Milestone

4.14

Development

Successfully merging this pull request may close these issues.

2 participants

@bentsku @joe4dev