Copilot commented on code in PR #64745:
URL: https://github.com/apache/airflow/pull/64745#discussion_r3300377889
##########
providers/amazon/src/airflow/providers/amazon/aws/operators/batch.py:
##########
@@ -330,18 +339,25 @@ def submit_job(self, context: Context):
job_id=self.job_id,
)
- def monitor_job(self, context: Context):
+ def _persist_links(
+ self,
+ context: Context,
+ job_description: dict | None = None,
+ ) -> dict:
Review Comment:
The new helper is typed as returning a bare `dict` and accepts a bare `dict
| None`. In this codebase the surrounding methods already use more specific
typing (e.g., `dict[str, Any]`). Consider updating this signature to
`job_description: Mapping[str, Any] | None` (or `dict[str, Any] | None`) and
returning `dict[str, Any]` to improve static checking and make the expected
structure clearer.
##########
providers/amazon/tests/unit/amazon/aws/operators/test_batch.py:
##########
@@ -149,16 +149,24 @@ def test_init_defaults(self):
def test_template_fields_overrides(self):
validate_template_fields(self.batch)
+ @mock.patch.object(BatchClientHook, "get_job_all_awslogs_info")
@mock.patch.object(BatchClientHook, "get_job_description")
@mock.patch.object(BatchClientHook, "wait_for_job")
@mock.patch.object(BatchClientHook, "check_job_success")
- def test_execute_without_failures(self, check_mock, wait_mock,
job_description_mock):
+ def test_execute_without_failures(
+ self, check_mock, wait_mock, job_description_mock,
get_job_all_awslogs_info_mock
+ ):
# JOB_ID is in RESPONSE_WITHOUT_FAILURES
self.client_mock.submit_job.return_value = RESPONSE_WITHOUT_FAILURES
self.batch.job_id = None
self.batch.waiters = None # use default wait
+ get_job_all_awslogs_info_mock.return_value = []
+ # Enable xcom push so _persist_cloudwatch_link actually runs
+ self.batch.do_xcom_push = True
+ # Use a real dict so "ti" in context works correctly
Review Comment:
These tests now depend on setting `do_xcom_push=True` to exercise
CloudWatch-link persistence. If link persistence is meant to be independent of
return-value XCom behavior (see comment on `_persist_cloudwatch_link`), the
tests should avoid coupling to `do_xcom_push` and instead only provide a
context with `ti`. Updating the tests accordingly will make them better reflect
the intended UI-link behavior.
--
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]
