Feature: Eventbridge v2: Add input path #10733

maxhoheiser · 2024-04-26T14:22:35Z

Motivation

Enable the input path (transformer) feature (https://docs.aws.amazon.com/eventbridge/latest/APIReference/API_Target.html).
This allows to specify an input path for the target, the body of matching event is parsed based on the specified json path and only the filtered content is sent to the target.

Changes

Add input transformer functionality to rule class.
Add process_event method that transforms the event content with the input path transformer if an input path is specified in the target.
Change from send_event to process_event method invoced by the provider.

github-actions · 2024-04-26T15:19:29Z

LocalStack Community integration with Pro

2 files ±0 2 suites ±0 1h 39m 10s ⏱️ + 2m 10s
2 935 tests +3 2 641 ✅ +3 294 💤 ±0 0 ❌ ±0
2 937 runs +3 2 641 ✅ +3 296 💤 ±0 0 ❌ ±0

Results for commit f05b155. ± Comparison against base commit 84f4a31a.

This pull request removes 1 and adds 4 tests. Note that renamed tests count towards both.

tests.aws.services.events.test_events_inputs.TestEventsInputPath ‑ test_put_events_with_input_path_multiple

tests.aws.services.events.test_events_inputs.TestEventsInputPath ‑ test_put_events_with_input_path_max_level_depth
tests.aws.services.events.test_events_inputs.TestEventsInputPath ‑ test_put_events_with_input_path_multiple_targets
tests.aws.services.events.test_events_inputs.TestEventsInputPath ‑ test_put_events_with_input_path_nested[event_detail0]
tests.aws.services.events.test_events_inputs.TestEventsInputPath ‑ test_put_events_with_input_path_nested[event_detail1]

♻️ This comment has been updated with latest results.

maxhoheiser · 2024-05-08T14:27:51Z

tests/aws/services/events/conftest.py


+ def _create_sqs_events_target(queue_name: str | None = None) -> tuple[str, str]:
+ if not queue_name:
+ queue_name = f"tests-queue-{short_uid()}"
 sqs_client = aws_client.sqs
 queue_url = sqs_client.create_queue(QueueName=queue_name)["QueueUrl"]


This on purpose does not use the sqs_create_queue fixture here, since it would need to be moved to helper functions because no cleanup and thus no factory would be required, which in turn would not valid this to be a fixture. This would in turn require passing in the aws_client as input.
I believe it is easier to read if the queue is created with the boot client here directly.

ok, fair point 👍

Why is no cleanup required here? Wouldn't the factory remove the need for an explicit cleanup after the yield here? It might have to do with the proper cleanup order 🤔

joe4dev

My main concern is regarding the introduction of an implicit transitive dependency and I'm not quite sure about the typings 😕
Otherwise, it looks good now 👍

joe4dev · 2024-05-08T08:51:15Z

localstack/services/events/target.py

 from localstack.utils.strings import to_bytes
 from localstack.utils.time import now_utc

 LOG = logging.getLogger(__name__)


+def transform_event_with_target_input_path(
+ input_path: TargetInputPath, event: PutEventsRequestEntry
+) -> PutEventsRequestEntry:


The type PutEventsRequestEntry might be misleading because the event has not much to do with a request here anymore 🤔

joe4dev · 2024-05-08T08:51:43Z

localstack/services/events/target.py

+def transform_event_with_target_input_path(
+ input_path: TargetInputPath, event: PutEventsRequestEntry
+) -> PutEventsRequestEntry:
+ event = extract_jsonpath(event, input_path)


nice utils reuse ✨

joe4dev · 2024-05-08T08:53:59Z

tests/aws/services/events/test_events_inputs.py

- Rule=rule_name,
- EventBusName=bus_name,
- Targets=[{"Id": target_id, "Arn": queue_arn, "InputPath": "$.detail"}],
+ @markers.aws.validated


joe4dev · 2024-05-08T08:56:40Z

tests/aws/services/events/test_events_integrations.py

@@ -634,9 +634,8 @@ def check_stream_status():
 assert_valid_event(data)


-@markers.aws.unknown
+@markers.aws.needs_fixing


Do we know what's broken here?

joe4dev · 2024-05-08T15:18:28Z

tests/aws/services/events/test_events_inputs.py

 {
- "Id": target_id_1,


good clarification 👍

super nit: using the same single-line formatting would make it more consistent and easier to read

joe4dev · 2024-05-08T15:22:41Z

tests/aws/services/events/conftest.py


+ def _create_sqs_events_target(queue_name: str | None = None) -> tuple[str, str]:
+ if not queue_name:
+ queue_name = f"tests-queue-{short_uid()}"
 sqs_client = aws_client.sqs
 queue_url = sqs_client.create_queue(QueueName=queue_name)["QueueUrl"]


ok, fair point 👍

Why is no cleanup required here? Wouldn't the factory remove the need for an explicit cleanup after the yield here? It might have to do with the proper cleanup order 🤔

joe4dev · 2024-05-08T18:10:01Z

localstack/services/events/models.py

-from typing import Optional
+from typing import Optional, TypeAlias
+
+from pydantic import BaseModel, Field


Is it worth introducing this new dependency here?

pydantic is a transitive dependency (through aws-sam-translator) and we should not rely on it without properly introducing it.

joe4dev · 2024-05-08T18:21:12Z

localstack/services/events/models.py

@@ -102,3 +106,20 @@ class InvalidEventPatternException(Exception):
 def __init__(self, reason=None, message=None) -> None:
 self.reason = reason
 self.message = message or f"Event pattern is not valid. Reason: {reason}"
+
+
+class FormattedEvent(BaseModel):


I'm not sure how much we gain by defining non-trivial types at this stage. Are we really sure this typing is correct?
It looks quite different than the previous type PutEventsRequestEntry, so at least one of these was/is quite wrong 😕

maxhoheiser added aws:events Amazon EventBridge semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases labels Apr 26, 2024

maxhoheiser requested a review from joe4dev April 26, 2024 14:22

maxhoheiser self-assigned this Apr 26, 2024

maxhoheiser force-pushed the feature/eventbridge_v2-add-input-path branch 4 times, most recently from 03803b1 to 76e25d8 Compare May 8, 2024 06:55

maxhoheiser marked this pull request as ready for review May 8, 2024 06:55

maxhoheiser requested review from dominikschubert and dfangl as code owners May 8, 2024 06:55

maxhoheiser force-pushed the feature/eventbridge_v2-add-input-path branch 3 times, most recently from 8efe19e to 9c8e0c6 Compare May 8, 2024 07:16

feat: add input path transformer

30131bc

maxhoheiser force-pushed the feature/eventbridge_v2-add-input-path branch 2 times, most recently from 50418c9 to 96012b4 Compare May 8, 2024 13:37

maxhoheiser added 8 commits May 8, 2024 16:19

feat: remove skip flag from succeeding tests

03ec1f9

feat: test validate input path

322f759

feat: test nested input path

f99a9e1

feat: test input path max level depth

7471f81

feat: test selective input path multiple targets

c921c42

feat: add type formatted event dict

c282704

feat: test duplicated key in event

e3437e5

feat: declare transformed return type

f05b155

maxhoheiser force-pushed the feature/eventbridge_v2-add-input-path branch from 968a98c to f05b155 Compare May 8, 2024 14:20

maxhoheiser commented May 8, 2024

View reviewed changes

joe4dev requested changes May 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Eventbridge v2: Add input path #10733

Feature: Eventbridge v2: Add input path #10733

maxhoheiser commented Apr 26, 2024 •

edited

github-actions bot commented Apr 26, 2024 •

edited

maxhoheiser May 8, 2024

joe4dev May 8, 2024

joe4dev left a comment

joe4dev May 8, 2024

joe4dev May 8, 2024

joe4dev May 8, 2024

joe4dev May 8, 2024

joe4dev May 8, 2024

joe4dev May 8, 2024

joe4dev May 8, 2024

joe4dev May 8, 2024

Feature: Eventbridge v2: Add input path #10733

Are you sure you want to change the base?

Feature: Eventbridge v2: Add input path #10733

Conversation

maxhoheiser commented Apr 26, 2024 • edited

Motivation

Changes

github-actions bot commented Apr 26, 2024 • edited

LocalStack Community integration with Pro

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joe4dev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxhoheiser commented Apr 26, 2024 •

edited

github-actions bot commented Apr 26, 2024 •

edited