SameerMesiah97 commented on code in PR #72449:
URL: https://github.com/apache/airflow/pull/72449#discussion_r3917174145
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/batch.py:
##########
@@ -102,6 +102,8 @@ def execute(self, context: Context) -> None:
job_id=self.job_id,
aws_conn_id=self.aws_conn_id,
region_name=self.region_name,
+ verify=self.verify,
+ botocore_config=self.botocore_config,
Review Comment:
I would advise you to run the prek hook locally to catch formatting errors
like this.
##########
providers/amazon/tests/unit/amazon/aws/sensors/test_batch.py:
##########
@@ -92,6 +92,28 @@ def test_execute_in_deferrable_mode(self,
deferrable_batch_sensor: BatchSensor):
deferrable_batch_sensor.execute({})
assert isinstance(exc.value.trigger, BatchJobTrigger), "Trigger is not
a BatchJobTrigger"
+ def test_execute_in_deferrable_mode_passes_aws_configs(self):
+ """Asserts that verify and botocore_config survive into the serialized
trigger payload."""
+ sensor = BatchSensor(
+ task_id="task",
+ job_id=JOB_ID,
+ region_name=AWS_REGION,
+ verify="/custom/ca_bundle.pem",
+ botocore_config={"read_timeout": 45},
+ deferrable=True,
+ )
+
+ with pytest.raises(TaskDeferred) as exc:
+ sensor.execute({})
+
+ trigger = exc.value.trigger
+ assert isinstance(trigger, BatchJobTrigger)
+
+ _, kwargs = trigger.serialize()
+ assert kwargs.get("region_name") == AWS_REGION
+ assert kwargs.get("verify") == "/custom/ca_bundle.pem"
+ assert kwargs.get("botocore_config") == {"read_timeout": 45}
Review Comment:
1) I believe that it would be better to test the actual values of the
attributes that materialize on the trigger to cover the forwarding behaviour
introduced by this change. Serialization is handled by `AwsBaseWaiterTrigger`
and should be covered by its own tests.
2) I dont think it is necessary to cover `region_name` as my understanding
is that the intention of this PR is to forward the 2 additional arguments i.e.
`verify` and `botocore_config`.
Below is my suggestion:
```
def test_execute_in_deferrable_mode_passes_aws_configs(self):
sensor = BatchSensor(
task_id="task",
job_id=JOB_ID,
verify="/custom/ca_bundle.pem",
botocore_config={"read_timeout": 45},
deferrable=True,
)
with pytest.raises(TaskDeferred) as exc:
sensor.execute({})
trigger = exc.value.trigger
assert isinstance(trigger, BatchJobTrigger)
assert trigger.verify == "/custom/ca_bundle.pem"
assert trigger.botocore_config == {"read_timeout": 45}
```
--
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]
