More human friendly "show tables" output for db cleanup (apache#38654)

idantepper · Apr 3, 2024 · 2ae3bbd · 2ae3bbd
1 parent 468ad72
commit 2ae3bbd
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 9 deletions.
diff --git a/airflow/utils/db_cleanup.py b/airflow/utils/db_cleanup.py
@@ -323,16 +323,18 @@ def _confirm_drop_archives(*, tables: list[str]):
     if len(tables) > 3:
         text_ = f"{len(tables)} archived tables prefixed with {ARCHIVE_TABLE_PREFIX}"
     else:
-        text_ = f"the following archived tables {tables}"
+        text_ = f"the following archived tables: {', '.join(tables)}"
     question = (
         f"You have requested that we drop {text_}.\n"
-        f"This is irreversible. Consider backing up the tables first \n"
+        f"This is irreversible. Consider backing up the tables first.\n"
     )
     print(question)
     if len(tables) > 3:
-        show_tables = ask_yesno("Show tables? (y/n): ")
+        show_tables = ask_yesno("Show tables that will be dropped? (y/n): ")
         if show_tables:
-            print(tables, "\n")
+            for table in tables:
+                print(f"  {table}")
+            print("\n")
     answer = input("Enter 'drop archived tables' (without quotes) to proceed.\n").strip()
     if answer != "drop archived tables":
         raise SystemExit("User did not confirm; exiting.")

diff --git a/tests/utils/test_db_cleanup.py b/tests/utils/test_db_cleanup.py
@@ -395,17 +395,17 @@ def test_confirm_drop_called_when_drop_archives_is_true_and_archive_exists(
     @patch("airflow.utils.db_cleanup.ask_yesno")
     def test_confirm_drop_archives(self, mock_ask_yesno, tables):
         expected = (
-            f"You have requested that we drop the following archived tables {tables}.\n"
-            "This is irreversible. Consider backing up the tables first"
+            f"You have requested that we drop the following archived tables: {', '.join(tables)}.\n"
+            "This is irreversible. Consider backing up the tables first."
         )
         if len(tables) > 3:
             expected = (
                 f"You have requested that we drop {len(tables)} archived tables prefixed with "
                 f"_airflow_deleted__.\n"
-                "This is irreversible. Consider backing up the tables first \n"
-                "\n"
-                f"{tables}"
+                "This is irreversible. Consider backing up the tables first.\n"
             )
+            for table in tables:
+                expected += f"\n  {table}"
 
         mock_ask_yesno.return_value = True
         with patch("sys.stdout", new=StringIO()) as fake_out, patch(