[GitHub] [airflow] ferruzzi commented on a diff in pull request #32869: Add Amazon EventBridge PutRule hook and operator

["ferruzzi (via GitHub)" <gi...@apache.org>]

ferruzzi commented on code in PR #32869:
URL: https://github.com/apache/airflow/pull/32869#discussion_r1275569763


##########
tests/system/providers/amazon/aws/example_eventbridge.py:
##########
@@ -45,6 +48,17 @@
 
     # [END howto_operator_eventbridge_put_events]
 
+    # [START howto_operator_eventbridge_put_rule]
+
+    put_rule = EventBridgePutRuleOperator(
+        task_id="put_rule_task",
+        name="Example Rule",
+        event_pattern='{"source": ["example.myapp"]}',
+        description="This rule matches events from example.myapp.",
+    )
+
+    # [END howto_operator_eventbridge_put_rule]
+
 

Review Comment:
   Now that you have more than one task in here you will want to add a chain() to set the order they run in.  Check out other system tests for examples



##########
airflow/providers/amazon/aws/hooks/eventbridge.py:
##########
@@ -16,12 +16,75 @@
 # under the License.
 from __future__ import annotations
 
+import json
+
+from airflow import AirflowException
 from airflow.providers.amazon.aws.hooks.base_aws import AwsBaseHook
+from airflow.providers.amazon.aws.utils import trim_none_values
+
+
+def validate_json(pattern: str) -> None:
+    try:
+        json.loads(pattern)
+    except ValueError:
+        raise AirflowException("Event pattern must be a valid JSON string.")
 
 
 class EventBridgeHook(AwsBaseHook):
     """Amazon EventBridge Hook."""
 
     def __init__(self, *args, **kwargs):
-        """Creating object."""
         super().__init__(client_type="events", *args, **kwargs)
+
+    def put_rule(
+        self,
+        name: str,
+        description: str | None = None,
+        event_bus_name: str | None = None,
+        event_pattern: str | None = None,
+        role_arn: str | None = None,
+        schedule_expression: str | None = None,
+        state: str | None = None,
+        tags: list[dict] | None = None,
+        **kwargs,
+    ):
+        """Create or update an EventBridge rule.
+

Review Comment:
   Here and elsewhere:   Don't worry about this unless you are already making other changes, but if you do make other changes, please add a line break there.  A future style rule will make us change that at some point:
   ```suggestion
           """
           Create or update an EventBridge rule.
   
   ```



##########
tests/providers/amazon/aws/operators/test_eventbridge.py:
##########
@@ -65,9 +67,39 @@ def test_failed_to_send(self, mock_conn: MagicMock):
         mock_conn.put_events.return_value = hook_response
 
         operator = EventBridgePutEventsOperator(
-            task_id=TASK_ID,
+            task_id="failed_put_events_job",
             entries=ENTRIES,
         )
 
         with pytest.raises(AirflowException):
             operator.execute(None)
+
+
+class TestEventBridgePutRuleOperator:
+    def test_init(self):
+        operator = EventBridgePutRuleOperator(
+            task_id="events_put_rule_job", name="match_s3_events", event_pattern=EVENT_PATTERN
+        )
+
+        assert operator.event_pattern == EVENT_PATTERN
+
+    @mock.patch.object(EventBridgeHook, "conn")
+    def test_execute(self, mock_conn: MagicMock):
+        hook_response = {"RuleArn": "arn:aws:events:us-east-1:123456789012:rule/test"}
+        mock_conn.put_rule.return_value = hook_response
+
+        operator = EventBridgePutRuleOperator(
+            task_id="events_put_rule_job", name="match_s3_events", event_pattern=EVENT_PATTERN
+        )
+
+        result = operator.execute(None)
+
+        assert result == hook_response
+
+    def test_put_rule_with_bad_json_fails(self):
+        operator = EventBridgePutRuleOperator(
+            task_id="failed_put_rule_job", name="match_s3_events", event_pattern='{"source"; ["aws.s3"]}'

Review Comment:
   I wonder...if this works as you'd expect, I wonder if just using any non-json string would work here and be more clear.  for example:
   
   ```suggestion
               task_id="failed_put_rule_job", name="match_s3_events", event_pattern='invalid json'
   ```
   
   If that works, IS it more clear, or should we leave it how it is?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org