8000 Remove legacy Lambda ESM v1 feature by joe4dev · Pull Request #11733 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content
8000

Remove legacy Lambda ESM v1 feature #11733

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 7, 2024
Merged

Remove legacy Lambda ESM v1 feature #11733

merged 1 commit into from
Nov 7, 2024

Conversation

joe4dev
Copy link
Member
@joe4dev joe4dev commented Oct 23, 2024 •
edited
Loading

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
Collaborator

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 GitHub Actions
Copy link
github-actions bot commented Oct 23, 2024 •
edited
Loading

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 GitHub Actions
Copy link
github-actions bot commented Oct 23, 2024 •
edited
Loading

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
This was referenced Nov 4, 2024
Merged
Merged
@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
gregfurman
gregfurman approved these changes Nov 5, 2024
View reviewed changes
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.

dfangl
dfangl approved these changes Nov 6, 2024
View reviewed changes
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
pinzon
pinzon approved these changes Nov 6, 2024
View reviewed changes
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
34 of 36 checks passed
@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
Merged
9 tasks
@gregfurman gregfurman mentioned this pull request Nov 7, 2024
Merged
alexrashed pushed a commit that referenced this pull request Nov 8, 2024
@joe4dev @alexrashed
Remove legacy Lambda ESM v1 feature (#11733)
660c9ba
bentsku added a commit that referenced this pull request Nov 8, 2024
@bentsku
fixup! Remove legacy Lambda ESM v1 feature (#11733)
7951399
joe4dev added a commit that referenced this pull request Nov 12, 2024
@joe4dev
Remove legacy Lambda ESM v1 feature (#11733)
fd6c627
joe4dev pushed a commit that referenced this pull request Nov 12, 2024
@bentsku @joe4dev
fixup! Remove legacy Lambda ESM v1 feature (#11733)
9c02521
dfangl pushed a commit that referenced this pull request Nov 14, 2024
@joe4dev @dfangl
Remove legacy Lambda ESM v1 feature (#11733)
03fcb57
@dfangl dfangl mentioned this pull request Nov 14, 2024
Merged
18 tasks
dominikschubert pushed a commit that referenced this pull request Nov 14, 2024
@joe4dev @dominikschubert
Remove legacy Lambda ESM v1 feature (#11733)
1eddb69
dominikschubert pushed a commit that referenced this pull request Nov 14, 2024
@joe4dev @dominikschubert
Remove legacy Lambda ESM v1 feature (#11733)
5ccf441
@joe4dev joe4dev mentioned this pull request Nov 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dfangl dfangl dfangl approved these changes

@pinzon pinzon pinzon approved these changes

@gregfurman gregfurman gregfurman approved these changes

@dominikschubert dominikschubert Awaiting requested review from dominikschubert

@simonrw simonrw Awaiting requested review from simonrw

@thrau thrau Awaiting requested review from thrau

Assignees

@joe4dev joe4dev

Labels
semver: major Breaking changes which can be included in a major release only
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.
5 participants
@joe4dev @localstack-bot @dfangl @pinzon @gregfurman
0