kakatur commented on PR #64745:
URL: https://github.com/apache/airflow/pull/64745#issuecomment-4271915595

   > Could you also please add some context to the description so we know 
what's being fixed, how we can tell it is fixed, etc
   
   **Fixes based on Copilot Comments:**
   
   1. Reduced API calls - 67% reduction (3→1) before deferring by removing 
redundant wait_for_job() call in execute_complete() (commit 0a6cbadd5c)
   2. Context validation - Added check for empty context (if not context or 
"ti" not in context) to skip CloudWatch API calls when context lacks task 
instance (commit 0058f4dc17)
   3. Removed XCom placeholder - Added if not self.do_xcom_push: return to 
respect do_xcom_push setting instead of always attempting to push CloudWatch 
links (commit 3b78fc97d4)
   4. Eliminated duplication in monitor_job - Extracted 
_persist_cloudwatch_link() helper method so CloudWatch info is fetched once 
instead of in multiple places (commit 3f17702130)
   5. Bug fix - Reordered code in execute_complete() to set job_id before 
persisting CloudWatch link, ensuring links work on both success and failure 
paths (commit 3b78fc97d4)
   6. Conditional logic based on awslogs_enabled - Initially gated CloudWatch 
links with if self.awslogs_enabled, then reverted to preserve UI links in all 
cases (commits 0058f4dc17, 4dc6b9d805)
   
   **Fixes based on Test Failures:**
   
   1. test_execute_without_failures - Updated to mock 
get_job_all_awslogs_info() instead of expecting multiple get_job_description() 
calls, enabled xcom_push, added proper dict context (commit e9b8d80dbf)
   2. test_execute_complete_success_with_logs - Added proper mocking of hook 
methods (get_job_all_awslogs_info, check_job_success) and enabled xcom_push 
with proper context (commit e9b8d80dbf)
   3. test_execute_complete_success_without_logs - Same mocking updates to 
handle CloudWatch link persistence in all scenarios (commit e9b8d80dbf)
   4. mypy type check / prek hook (raise AirflowException) - Replaced 
AirflowException with ValueError for missing job_id, added type guard for 
job_id in _persist_cloudwatch_link() (commit 9b210efb34)


-- 
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]

Reply via email to