SameerMesiah97 commented on code in PR #73215:
URL: https://github.com/apache/airflow/pull/73215#discussion_r4027627532
##########
providers/amazon/tests/unit/amazon/aws/triggers/test_opensearch_serverless.py:
##########
@@ -138,3 +138,41 @@ def
test_status_queries_surface_collection_error_details(self):
}
assert str(_LazyStatusFormatter(trigger.status_queries, not_found)) ==
NOT_FOUND_MESSAGE
+ @pytest.mark.parametrize(
+ ("collection_name", "collection_id"),
+ [
+ pytest.param(COLLECTION_NAME, None, id="collection_name"),
+ pytest.param(None, COLLECTION_ID, id="collection_id"),
+ ],
+)
+ def test_aws_config_serialization(self, collection_name, collection_id):
+ trigger = OpenSearchServerlessCollectionActiveTrigger(
+ collection_name=collection_name,
+ collection_id=collection_id,
+ aws_conn_id="aws-test-custom-conn",
+ region_name="eu-west-1",
+ verify=False,
+ botocore_config={"read_timeout": 42},
+ )
+
+ _, kwargs = trigger.serialize()
+ assert kwargs["aws_conn_id"] == "aws-test-custom-conn"
+ assert kwargs["region_name"] == "eu-west-1"
+ assert kwargs["verify"] is False
+ assert kwargs["botocore_config"] == {"read_timeout": 42}
Review Comment:
There is already a parametrized serialization test covering both the
collection-name and collection-ID paths. Could we extend that test with the
newly supported AWS configuration instead of introducing a second test covering
the same two paths?
##########
providers/amazon/tests/unit/amazon/aws/sensors/test_opensearch_serverless.py:
##########
@@ -102,3 +102,22 @@ def test_poke_failure_states(self, mock_conn, state):
sensor =
OpenSearchServerlessCollectionActiveSensor(**self.default_op_kwargs,
aws_conn_id=None)
with pytest.raises(AirflowException, match=sensor.FAILURE_MESSAGE):
sensor.poke({})
Review Comment:
There should be a blank line here.
##########
providers/amazon/tests/unit/amazon/aws/triggers/test_opensearch_serverless.py:
##########
@@ -138,3 +138,41 @@ def
test_status_queries_surface_collection_error_details(self):
}
assert str(_LazyStatusFormatter(trigger.status_queries, not_found)) ==
NOT_FOUND_MESSAGE
+ @pytest.mark.parametrize(
+ ("collection_name", "collection_id"),
+ [
+ pytest.param(COLLECTION_NAME, None, id="collection_name"),
+ pytest.param(None, COLLECTION_ID, id="collection_id"),
+ ],
+)
+ def test_aws_config_serialization(self, collection_name, collection_id):
+ trigger = OpenSearchServerlessCollectionActiveTrigger(
+ collection_name=collection_name,
+ collection_id=collection_id,
+ aws_conn_id="aws-test-custom-conn",
+ region_name="eu-west-1",
+ verify=False,
+ botocore_config={"read_timeout": 42},
+ )
+
+ _, kwargs = trigger.serialize()
+ assert kwargs["aws_conn_id"] == "aws-test-custom-conn"
+ assert kwargs["region_name"] == "eu-west-1"
+ assert kwargs["verify"] is False
+ assert kwargs["botocore_config"] == {"read_timeout": 42}
+ def test_hook_uses_aws_config(self):
+ trigger = OpenSearchServerlessCollectionActiveTrigger(
+ collection_id=self.COLLECTION_ID,
+ aws_conn_id="aws-test-custom-conn",
+ region_name="eu-west-1",
+ verify=False,
+ botocore_config={"read_timeout": 42},
+ )
+
+ hook = trigger.hook()
+
+ assert hook.aws_conn_id == "aws-test-custom-conn"
+ assert hook._region_name == "eu-west-1"
+ assert hook._verify is False
+ assert hook._config is not None
+ assert hook._config.read_timeout == 42
Review Comment:
You are currently inspecting the private attributes of the hook. Would be
better to patch the hook itself and verify the arguments for each parameter in
the hook constructor. Please see the below:
```
@mock.patch(TRIGGER_MODULE + ".OpenSearchServerlessHook")
def test_hook_uses_aws_config(self, mock_hook):
trigger = OpenSearchServerlessCollectionActiveTrigger(
collection_id=self.COLLECTION_ID,
aws_conn_id="aws-test-custom-conn",
region_name="eu-west-1",
verify=False,
botocore_config={"read_timeout": 42},
)
assert trigger.hook() == mock_hook.return_value
mock_hook.assert_called_once_with(
aws_conn_id="aws-test-custom-conn",
region_name="eu-west-1",
verify=False,
config={"read_timeout": 42},
)
```
##########
providers/amazon/src/airflow/providers/amazon/aws/triggers/opensearch_serverless.py:
##########
@@ -46,14 +46,16 @@ def __init__(
waiter_delay: int = 60,
waiter_max_attempts: int = 20,
aws_conn_id: str | None = None,
+ region_name: str | None = None,
+ verify: bool | str | None = None,
+ botocore_config: dict | None = None,
Review Comment:
Indentation is off here and in other function signatures. Please run the
prek hook to fix it.
##########
providers/amazon/src/airflow/providers/amazon/aws/triggers/opensearch_serverless.py:
##########
@@ -46,14 +46,16 @@ def __init__(
waiter_delay: int = 60,
waiter_max_attempts: int = 20,
aws_conn_id: str | None = None,
+ region_name: str | None = None,
+ verify: bool | str | None = None,
+ botocore_config: dict | None = None,
) -> None:
if not exactly_one(collection_id is None, collection_name is None):
raise AttributeError("Either collection_ids or collection_names
must be provided, not both.")
super().__init__(
serialized_fields={"collection_id": collection_id,
"collection_name": collection_name},
waiter_name="collection_available",
- # waiter_args is a dict[str, Any], allow a possible list of None
(it is caught above)
Review Comment:
Why was this comment removed? Could you add it back?
##########
providers/amazon/src/airflow/providers/amazon/aws/triggers/opensearch_serverless.py:
##########
Review Comment:
Could you add the new paramters to the docstring?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
