Changed revert report to report on all "what" types rather than managed/external tables (#924)

Author: FastLee

src/databricks/labs/ucx/hive_metastore/table_migrate.py

@@ -153,25 +153,12 @@ def _get_revert_count(self, schema: str | None = None, table: str | None = None)
 
         migration_list = []
         for cur_database, tables in table_by_database.items():
-            external_tables = 0
-            managed_tables = 0
-            views = 0
+            what_count: dict[What, int] = {}
             for current_table in tables:
                 if current_table.upgraded_to is not None:
-                    if current_table.kind == "VIEW":
-                        views += 1
-                        continue
-                    if current_table.object_type == "EXTERNAL":
-                        external_tables += 1
-                        continue
-                    if current_table.object_type == "MANAGED":
-                        managed_tables += 1
-                        continue
-            migration_list.append(
-                MigrationCount(
-                    database=cur_database, managed_tables=managed_tables, external_tables=external_tables, views=views
-                )
-            )
+                    count = what_count.get(current_table.what, 0)
+                    what_count[current_table.what] = count + 1
+            migration_list.append(MigrationCount(database=cur_database, what_count=what_count))
         return migration_list
 
     def is_upgraded(self, schema: str, table: str) -> bool:
@@ -189,17 +176,36 @@ def print_revert_report(self, *, delete_managed: bool) -> bool | None:
             logger.info("No migrated tables were found.")
             return False
         print("The following is the count of migrated tables and views found in scope:")
-        print("Database                      | External Tables  | Managed Table    | Views            |")
-        print("=" * 88)
+        table_header = "Database            |"
+        headers = 1
+        for what in list(What):
+            headers = max(headers, len(what.name.split("_")))
+            table_header += f" {what.name.split('_')[0]:<10} |"
+        print(table_header)
+        # Split the header so _ separated what names are splitted into multiple lines
+        for header in range(1, headers):
+            table_sub_header = "                    |"
+            for what in list(What):
+                if len(what.name.split("_")) - 1 < header:
+                    table_sub_header += f"{' '*12}|"
+                    continue
+                table_sub_header += f" {what.name.split('_')[header]:<10} |"
+            print(table_sub_header)
+        separator = "=" * (22 + 13 * len(What))
+        print(separator)
         for count in migrated_count:
-            print(f"{count.database:<30}| {count.external_tables:16} | {count.managed_tables:16} | {count.views:16} |")
-        print("=" * 88)
-        print("Migrated External Tables and Views (targets) will be deleted")
+            table_row = f"{count.database:<20}|"
+            for what in list(What):
+                table_row += f" {count.what_count.get(what, 0):10} |"
+            print(table_row)
+        print(separator)
+        print("The following actions will be performed")
+        print("- Migrated External Tables and Views (targets) will be deleted")
         if delete_managed:
-            print("Migrated Manged Tables (targets) will be deleted")
+            print("- Migrated DBFS Root Tables will be deleted")
         else:
-            print("Migrated Manged Tables (targets) will be left intact.")
-            print("To revert and delete Migrated Tables, add --delete_managed true flag to the command.")
+            print("- Migrated DBFS Root Tables will be left intact")
+            print("To revert and delete Migrated Tables, add --delete_managed true flag to the command")
         return True
 
 

src/databricks/labs/ucx/hive_metastore/tables.py

@@ -147,9 +147,7 @@ class TableError:
 @dataclass
 class MigrationCount:
     database: str
-    managed_tables: int = 0
-    external_tables: int = 0
-    views: int = 0
+    what_count: dict[What, int]
 
 
 class TablesCrawler(CrawlerBase):

tests/unit/hive_metastore/test_table_migrate.py

@@ -238,6 +238,7 @@ def get_table_migrate(backend: SqlBackend) -> TablesMigrate:
             catalog="hive_metastore",
             database="test_schema1",
             name="test_table1",
+            location="s3://some_location/table",
             upgraded_to="cat1.schema1.dest1",
         ),
         Table(
@@ -255,6 +256,7 @@ def get_table_migrate(backend: SqlBackend) -> TablesMigrate:
             catalog="hive_metastore",
             database="test_schema1",
             name="test_table2",
+            location="dbfs:/dbfs_location/table",
             upgraded_to="cat1.schema1.dest2",
         ),
         Table(
@@ -263,6 +265,7 @@ def get_table_migrate(backend: SqlBackend) -> TablesMigrate:
             catalog="hive_metastore",
             database="test_schema2",
             name="test_table3",
+            location="s3://some_location/table",
             upgraded_to="cat1.schema2.dest3",
         ),
     ]
@@ -352,8 +355,13 @@ def test_get_migrated_count():
     backend = MockBackend(fails_on_first=errors, rows=rows)
     table_migrate = get_table_migrate(backend)
     migrated_count = table_migrate._get_revert_count()
-    assert MigrationCount("test_schema1", 1, 1, 1) in migrated_count
-    assert MigrationCount("test_schema2", 0, 1, 0) in migrated_count
+    assert (
+        MigrationCount(
+            database="test_schema1", what_count={What.VIEW: 1, What.DBFS_ROOT_DELTA: 1, What.EXTERNAL_SYNC: 1}
+        )
+        in migrated_count
+    )
+    assert MigrationCount("test_schema2", {What.EXTERNAL_SYNC: 1}) in migrated_count
 
 
 def test_revert_report(capsys):
@@ -363,13 +371,13 @@ def test_revert_report(capsys):
     table_migrate = get_table_migrate(backend)
     table_migrate.print_revert_report(delete_managed=True)
     captured = capsys.readouterr()
-    assert "test_schema1|1|1|1" in captured.out.replace(" ", "")
-    assert "test_schema2|1|0|0" in captured.out.replace(" ", "")
-    assert "Migrated Manged Tables (targets) will be deleted" in captured.out
+    assert "test_schema1|1|0|1|0|1|0|0|" in captured.out.replace(" ", "")
+    assert "test_schema2|1|0|0|0|0|0|0|" in captured.out.replace(" ", "")
+    assert "- Migrated DBFS Root Tables will be deleted" in captured.out
 
     table_migrate.print_revert_report(delete_managed=False)
     captured = capsys.readouterr()
-    assert "Migrated Manged Tables (targets) will be left intact" in captured.out
+    assert "- Migrated DBFS Root Tables will be left intact" in captured.out
 
 
 def test_empty_revert_report(capsys):