kevinjqliu commented on code in PR #947:
URL: https://github.com/apache/iceberg-python/pull/947#discussion_r1685511390


##########
mkdocs/docs/configuration.md:
##########
@@ -139,7 +139,13 @@ For the FileIO there are several configuration options 
available:
 
 ## Catalogs
 
-PyIceberg currently has native support for REST, SQL, Hive, Glue and DynamoDB.
+PyIceberg currently has native catalog type support for REST, SQL, Hive, Glue 
and DynamoDB.
+You can also set the catalog implementation explicitly:
+
+| Key             | Example                      | Description                 
                                                                     |
+| --------------- | ---------------------------- | 
------------------------------------------------------------------------------------------------
 |
+| type            | rest                         | Type of catalog, one of 
`rest`, `sql`, `hive`, `glue`, `dymamodb`. Default to `rest`             |

Review Comment:
   nit: I think it'll be good to convey the either/or relation here



##########
pyiceberg/catalog/__init__.py:
##########
@@ -283,6 +292,20 @@ def delete_data_files(io: FileIO, manifests_to_delete: 
List[ManifestFile]) -> No
                 deleted_files[path] = True
 
 
+def _import_catalog(name: str, catalog_impl: str, properties: Properties) -> 
Optional[Catalog]:
+    try:
+        path_parts = catalog_impl.split(".")
+        if len(path_parts) < 2:
+            raise ValueError(f"py-catalog-impl should be full path 
(module.CustomCatalog), got: {catalog_impl}")
+        module_name, class_name = ".".join(path_parts[:-1]), path_parts[-1]
+        module = importlib.import_module(module_name)
+        class_ = getattr(module, class_name)
+        return class_(name, **properties)

Review Comment:
   nit: should we also try to catch any initialization errors? 



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to