Skip to content

Remove legacy Lambda ESM v1 feature#11733

Merged
joe4dev merged 1 commit intorelease/v4from
remove-legacy-event-source-mapping-feature
Nov 7, 2024
Merged

Remove legacy Lambda ESM v1 feature#11733
joe4dev merged 1 commit intorelease/v4from
remove-legacy-event-source-mapping-feature

Conversation

@joe4dev
Copy link
Member

@joe4dev joe4dev commented Oct 23, 2024

Depends on CI step removal #11779

Motivation

The new Lambda Event Source Mapping (ESM) implementation v2 has been the default implementation since 3.8.0 and we announced the removal of ESM v1 with the next major release in the release notes.

Changes

  • Remove config LAMBDA_EVENT_SOURCE_MAPPING and all dependencies
  • Add LAMBDA_EVENT_SOURCE_MAPPING to deprecations
  • Remove conditional deprecation for value of LAMBDA_EVENT_SOURCE_MAPPING
  • Remove code localstack-core/localstack/services/lambda_/event_source_listeners
  • Refactor utils used in ESM v2 → extract methods into ESM v2
    • from localstack.services.lambda_.event_source_listeners.utils import message_attributes_to_lower
    • from localstack.services.lambda_.event_source_listeners.utils import ( has_data_filter_criteria_parsed,)
  • Deprecate and remove LAMBDA_SQS_EVENT_SOURCE_MAPPING_INTERVAL_SEC (we should have deprecated it earlier, but it's undocumented)

Testing

Mostly regression test suite.

Could check deprecation warnings manually.

TODO

What's left to do:

  • Update Lambda v4 docs accordingly (near the release)

Discussion

  • Should we consider switching the default event filter engine to Java (JPype-based)?
    • Pros
      • Perfect AWS parity for event filtering demonstrated by the test suite tests/aws/services/events/test_events_patterns.py (summary of different implementations in Add event matching test suite #10599)
    • Cons
      • Slower than Python-native (usually in the 10s on microseconds, but highly variable with spikes up to ~150-200ms)
      • Potential ext pipeline interaction with the siddy library used for Kinesisanalytics (same problem for EventBridge v2) and already monkeypatched ESM v2 tests
    • Dependency: The Python-based rule engine uses the same implementation as EventBridge v1

@joe4dev joe4dev added the semver: major Breaking changes which can be included in a major release only label Oct 23, 2024
@joe4dev joe4dev added this to the 4.0 milestone Oct 23, 2024
@joe4dev joe4dev self-assigned this Oct 23, 2024
@localstack-bot
Copy link
Contributor

Currently, only minor and patch changes are allowed on master. Your PR labels (semver: major) indicate that it cannot be merged into the master at this time.

@joe4dev joe4dev changed the title Remove legacy Lambda ESM v1 Remove legacy Lambda ESM v1 feature Oct 23, 2024
@github-actions
Copy link

github-actions bot commented Oct 23, 2024

S3 Image Test Results (AMD64 / ARM64)

  2 files    2 suites   3m 34s ⏱️
421 tests 369 ✅  52 💤 0 ❌
842 runs  738 ✅ 104 💤 0 ❌

Results for commit ede91bb.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 23, 2024

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 44m 27s ⏱️ + 2m 24s
3 532 tests ±0  3 118 ✅ ±0  414 💤 ±0  0 ❌ ±0 
3 534 runs  ±0  3 118 ✅ ±0  416 💤 ±0  0 ❌ ±0 

Results for commit 9301dba. ± Comparison against base commit efc629f.

♻️ This comment has been updated with latest results.

@joe4dev joe4dev force-pushed the remove-legacy-event-source-mapping-feature branch from 2fe8fd5 to 7974616 Compare October 24, 2024 11:19
@joe4dev joe4dev force-pushed the remove-legacy-event-source-mapping-feature branch from 7974616 to 9301dba Compare November 5, 2024 08:30
@joe4dev joe4dev changed the base branch from master to release/v4 November 5, 2024 08:36
@joe4dev joe4dev marked this pull request as ready for review November 5, 2024 08:51
Copy link
Contributor

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Mostly nits and questions. Anything suggested can easily be done in a follow-up or ignored.

Copy link
Member

@dfangl dfangl left a comment

Choose a reason for hiding this comment

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

LGTM, nice removal!

@dfangl dfangl force-pushed the remove-legacy-event-source-mapping-feature branch from 9301dba to ede91bb Compare November 6, 2024 10:01
Copy link
Member

@pinzon pinzon left a comment

Choose a reason for hiding this comment

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

Awesome!

@joe4dev joe4dev merged commit f093a54 into release/v4 Nov 7, 2024
@joe4dev joe4dev deleted the remove-legacy-event-source-mapping-feature branch November 7, 2024 09:12
@alexrashed alexrashed mentioned this pull request Nov 7, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver: major Breaking changes which can be included in a major release only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants