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 36m 42s ⏱️ +54s
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 b9d2dd3. ± Comparison against base commit cbec2f4.

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 👍

localstack/services/events/target.py

tests/aws/services/events/test_events_inputs.py

tests/aws/services/events/test_events_integrations.py

tests/aws/services/events/test_events_inputs.py

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 🤔

localstack/services/events/models.py

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 😕

It is quite different since the initial defined type was not accurate. We transform the incoming event (here: https://github.com/localstack/localstack/blob/feature/eventbridge_v2-add-input-path/localstack/services/events/provider.py#L129) and add new fields since they are expected by the client.

joe4dev

LGTM 👍

Thanks for removing the implicit pydantic dependency.

joe4dev · 2024-05-14T15:56:34Z

localstack/services/events/models.py

+class FormattedEvent(TypedDict):
+    version: str
+    id: str
+    detail_type: Optional[str]  # key "detail-type" is automatically interpreted as detail_type


@maxhoheiser This type definition allows both detail-type and detail_type, but we should restrict it to the hyphen-only variant.

Python supports an alternative syntax for defining TypedDicts, which is recommended in such cases:

The functional syntax should also be used when any of the keys are not valid identifiers, for example because they are keywords or contain hyphens.
https://docs.python.org/3/library/typing.html#typing.TypedDict

All credits go to @dfangl for this suggestion 💯

joe4dev · 2024-05-14T16:00:44Z

localstack/services/events/models.py

+    detail: dict[str, str | dict]
+
+
+TransformedEvent: TypeAlias = FormattedEvent | dict | str


I can understand why we end up such a flexible type definition given the many possibilities using input templates. For readers, this type definition is quite confusing. Consider adding a comment for now.
Maybe, we find something clearer in the future to clarify the type after each step without too many XORs (especially in methods).

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 6 times, most recently 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

maxhoheiser force-pushed the feature/eventbridge_v2-add-input-path branch from f05b155 to 31269bf Compare May 13, 2024 08:08

maxhoheiser added 5 commits May 13, 2024 10:08

feat: add input path transformer

1d50936

feat: remove skip flag from succeeding tests

279f03a

feat: test validate input path

782930e

feat: test nested input path

f33c015

feat: test input path max level depth

0984224

maxhoheiser force-pushed the feature/eventbridge_v2-add-input-path branch from 31269bf to 876a1be Compare May 13, 2024 08:08

feat: test selective input path multiple targets

07951a2

maxhoheiser force-pushed the feature/eventbridge_v2-add-input-path branch from 876a1be to d0131f3 Compare May 13, 2024 08:15

maxhoheiser requested a review from joe4dev May 13, 2024 08:17

maxhoheiser added 3 commits May 13, 2024 12:53

feat: add type formatted event dict

da96b7c

feat: test duplicated key in event

bc5d39a

feat: declare transformed return type

b9d2dd3

maxhoheiser force-pushed the feature/eventbridge_v2-add-input-path branch from d0131f3 to b9d2dd3 Compare May 13, 2024 10:53

joe4dev approved these changes May 13, 2024

View reviewed changes

maxhoheiser merged commit cdb940e into master May 13, 2024
14 checks passed

maxhoheiser deleted the feature/eventbridge_v2-add-input-path branch May 13, 2024 12:00

joe4dev reviewed May 14, 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

maxhoheiser May 13, 2024

joe4dev left a comment

joe4dev May 14, 2024

joe4dev May 14, 2024

		detail: dict[str, str \| dict]


		TransformedEvent: TypeAlias = FormattedEvent \| dict \| str

Feature: Eventbridge v2: Add input path #10733

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

maxhoheiser May 8, 2024

Choose a reason for hiding this comment

joe4dev May 8, 2024

Choose a reason for hiding this comment

joe4dev left a comment

Choose a reason for hiding this comment

joe4dev May 8, 2024

Choose a reason for hiding this comment

joe4dev May 8, 2024

Choose a reason for hiding this comment

maxhoheiser May 13, 2024

Choose a reason for hiding this comment

joe4dev left a comment

Choose a reason for hiding this comment

joe4dev May 14, 2024

Choose a reason for hiding this comment

joe4dev May 14, 2024

Choose a reason for hiding this comment

maxhoheiser commented Apr 26, 2024 •

edited

github-actions bot commented Apr 26, 2024 •

edited