ferruzzi commented on code in PR #64745:
URL: https://github.com/apache/airflow/pull/64745#discussion_r3230319917
##########
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:
"""
- Monitor an AWS Batch job.
+ Persist job definition and queue links for UI display.
- This can raise an exception or an AirflowTaskTimeout if the task was
- created with ``execution_timeout``.
+ :param context: Task context
+ :param job_description: Optional pre-fetched job description to avoid
redundant API calls
+ :return: Job description dict
"""
if not self.job_id:
raise AirflowException("AWS Batch job - job_id was not found")
Review Comment:
I know this is code you didn't touch, but I agree with you using ValueError
in your new code and this feels like it should also be a ValueError. IIRC
AirflowException is more for runtime issues, not "we didn't pass a value"
errors.
Feel free to just resolve this if you don't want to handle it. Just a
thought.
##########
providers/amazon/src/airflow/providers/amazon/aws/operators/batch.py:
##########
@@ -252,14 +258,17 @@ def execute(self, context: Context) -> str | None:
def execute_complete(self, context: Context, event: dict[str, Any] | None
= None) -> str:
validated_event = validate_execute_complete_event(event)
+ # Set job_id first so CloudWatch link can be persisted even on failure
+ self.job_id = validated_event["job_id"]
+
+ # Persist CloudWatch logs for both success and failure
+ self._persist_cloudwatch_link(context)
Review Comment:
Consider wrapping this in an "if cloudwatch..." check to save the API call?
--
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]
