8000 Remove legacy event filtering implementation by joe4dev · Pull Request #11985 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content

Remove legacy event filtering implementation #11985

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 2 commits into from
Dec 4, 2024
Merged

Remove legacy event filtering implementation #11985

merged 2 commits into from
Dec 4, 2024

Conversation

joe4dev
Copy link
Member
@joe4dev joe4dev commented Dec 4, 2024

Motivation

With @bentsku 's amazing new Python-native event-filtering engine introduced in #11960, the old ESM v1 implementation is obsolete and can be removed 🧹 .

Changes

Remove legacy ESM v1 event filtering Python implementation

@joe4dev joe4dev added the semver: patch Non-breaking changes which can be included in patch releases label Dec 4, 2024
@joe4dev joe4dev requested a review from bentsku December 4, 2024 14:27
@joe4dev joe4dev self-assigned this Dec 4, 2024
bentsku
bentsku reviewed Dec 4, 2024
View reviewed changes
Copy link
Contributor
@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

Awesome cleanup! LGTM for this part!

It might worth removing the code in localstack.services.events.v1.utils too, especially code related to matches_event.

Should this be part of this PR too?

@joe4dev joe4dev requested a review from maxhoheiser as a code owner December 4, 2024 15:04
@joe4dev
Copy link
Member Author
joe4dev commented Dec 4, 2024

Awesome cleanup! LGTM for this part!

It might worth removing the code in localstack.services.events.v1.utils too, especially code related to matches_event.

Should this be part of this PR too?

Yes, +1 🧹

Thanks for pointing this out @bentsku

bentsku
bentsku approved these changes Dec 4, 2024
View reviewed changes
Copy link
Contributor
@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for cleaning up 🧹 seems like there were 0 usages of this anywhere as it was nicely refactored to be used in a single place in localstack/utils/event_matcher.py by @zaingz 👌

@github-actions GitHub Actions
Copy link
github-actions bot commented Dec 4, 2024

LocalStack Community integration with Pro

    2 files  ±  0      2 suites  ±0   1h 27m 32s ⏱️ - 23m 36s
2 880 tests  - 946  2 659 ✅  - 849  221 💤  - 97  0 ❌ ±0 
2 882 runs   - 946  2 659 ✅  - 849  223 💤  - 97  0 ❌ ±0 

Results for commit 00eb178. ± Comparison against base commit 35cb30e.

This pull request removes 946 tests. 
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]
…

@joe4dev joe4dev merged commit e0638d4 into master Dec 4, 2024
31 checks passed
@joe4dev joe4dev deleted the cleanup-unused-event-filtering-code branch December 4, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bentsku bentsku bentsku approved these changes

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

@dfangl dfangl Awaiting requested review from dfangl dfangl is a code owner

@gregfurman gregfurman Awaiting requested review from gregfurman gregfurman is a code owner

@maxhoheiser maxhoheiser Awaiting requested review from maxhoheiser maxhoheiser is a code owner

Assignees

@joe4dev joe4dev

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@joe4dev @bentsku
0