rymurr commented on a change in pull request #2266:
URL: https://github.com/apache/iceberg/pull/2266#discussion_r604694415



##########
File path: python/iceberg/hive/hive_tables.py
##########
@@ -34,12 +40,41 @@ def __init__(self, conf):
     def new_table_ops(self, conf, database, table):
         return HiveTableOperations(conf, self.get_client(), database, table)
 
-    def drop(self, database, table):
-        raise RuntimeError("Not yet implemented")
+    def drop(self, database: str, table: str, purge: bool = False) -> None:
+        ops = self.new_table_ops(self.conf, database, table)
+        metadata = ops.current()
 
-    def get_client(self):
+        with self.get_client() as open_client:
+            _logger.info("Deleting {database}.{table} from Hive 
Metastore".format(database=database, table=table))
+            open_client.drop_table(database, table, deleteData=False)
+
+        if purge:
+            if metadata is not None:
+                with Pool(self.conf.get(WORKER_THREAD_POOL_SIZE_PROP,
+                                        cpu_count())) as delete_pool:
+                    for s in metadata.snapshots:
+                        for m in s.manifests:
+                            delete_pool.map(self._delete_file(ops),
+                                            (i.path() for i in 
s.get_filtered_manifest(m.manifest_path).iterator()))
+                        delete_pool.map(self._delete_file(ops), 
(m.manifest_path for m in s.manifests))
+                        if s.manifest_location is not None:
+                            delete_pool.map(self._delete_file(ops), 
[s.manifest_location])
+                    delete_pool.map(self._delete_file(ops), 
[ops.current_metadata_location])
+
+    def get_client(self) -> HMSClient:
         from urllib.parse import urlparse
         metastore_uri = urlparse(self.conf[HiveTables.THRIFT_URIS])
 
         client = hmsclient.HMSClient(host=metastore_uri.hostname, 
port=metastore_uri.port)
         return client
+
+    @staticmethod
+    def _delete_file(ops: HiveTableOperations) -> Callable[[str], None]:
+        def _delete_file_internal(path: str) -> None:

Review comment:
       I think it might be nicer to call `self._delete_file` once in the above 
method and add a `set` here. Then log only files that aren't in the set yet.

##########
File path: python/iceberg/hive/hive_tables.py
##########
@@ -34,12 +40,41 @@ def __init__(self, conf):
     def new_table_ops(self, conf, database, table):
         return HiveTableOperations(conf, self.get_client(), database, table)
 
-    def drop(self, database, table):
-        raise RuntimeError("Not yet implemented")
+    def drop(self, database: str, table: str, purge: bool = False) -> None:
+        ops = self.new_table_ops(self.conf, database, table)
+        metadata = ops.current()
 
-    def get_client(self):
+        with self.get_client() as open_client:
+            _logger.info("Deleting {database}.{table} from Hive 
Metastore".format(database=database, table=table))
+            open_client.drop_table(database, table, deleteData=False)
+
+        if purge:
+            if metadata is not None:
+                with Pool(self.conf.get(WORKER_THREAD_POOL_SIZE_PROP,
+                                        cpu_count())) as delete_pool:
+                    for s in metadata.snapshots:
+                        for m in s.manifests:
+                            delete_pool.map(self._delete_file(ops),
+                                            (i.path() for i in 
s.get_filtered_manifest(m.manifest_path).iterator()))
+                        delete_pool.map(self._delete_file(ops), 
(m.manifest_path for m in s.manifests))
+                        if s.manifest_location is not None:
+                            delete_pool.map(self._delete_file(ops), 
[s.manifest_location])
+                    delete_pool.map(self._delete_file(ops), 
[ops.current_metadata_location])
+
+    def get_client(self) -> HMSClient:
         from urllib.parse import urlparse
         metastore_uri = urlparse(self.conf[HiveTables.THRIFT_URIS])
 
         client = hmsclient.HMSClient(host=metastore_uri.hostname, 
port=metastore_uri.port)
         return client
+
+    @staticmethod
+    def _delete_file(ops: HiveTableOperations) -> Callable[[str], None]:
+        def _delete_file_internal(path: str) -> None:
+            _logger.info("Deleting file: {path}".format(path=path))
+            try:
+                ops.delete_file(path)
+            except OSError as e:
+                _logger.info("Error deleting file: {path}: 
{e}".format(path=path, e=e))
+                pass

Review comment:
       nit: this `pass` is redundant now




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

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to