hkc-8010 commented on code in PR #65239:
URL: https://github.com/apache/airflow/pull/65239#discussion_r3104609948


##########
airflow-core/src/airflow/utils/db_cleanup.py:
##########
@@ -613,9 +631,22 @@ def run_cleanup(
                     batch_size=batch_size,
                 )
                 session.commit()
+            if ctx.failed:
+                failed_tables.append(table_name)
         else:
             logger.warning("Table %s not found.  Skipping.", table_name)
 
+    if failed_tables:
+        logger.warning(

Review Comment:
   Done — restructured so the warning is only emitted when 
`error_on_cleanup_failure` is `False`. When the flag is set, the `RuntimeError` 
message already lists the failed tables, so no duplicate output.



##########
airflow-core/tests/unit/utils/test_db_cleanup.py:
##########
@@ -741,6 +741,67 @@ def test_drop_archived_tables(self, mock_input, 
confirm_mock, inspect_mock, capl
         else:
             confirm_mock.assert_not_called()
 
+    @patch(
+        "airflow.utils.db_cleanup._cleanup_table",
+        side_effect=OperationalError("", {}, Exception("mock db error")),
+    )
+    def test_error_on_cleanup_failure_raises_when_flag_set(self, 
cleanup_table_mock, caplog):

Review Comment:
   Done — replaced all `caplog` usage in the affected tests with 
`patch('airflow.utils.db_cleanup.logger')` and `assert_any_call` assertions 
instead.



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