From b8aed8d6ebfceff57b31ee99ce46ca2bee810de0 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sat, 27 Dec 2025 18:40:43 -0800 Subject: [PATCH 1/6] Clean up logging: move exception tracebacks to debug level Move exception info from warning to debug level in import error handlers. This provides cleaner user-facing output by default while still making detailed exception info available for troubleshooting via debug logging. Changes: - FileIO import failures (pyiceberg.io.__init__) - LocationProvider import failures (pyiceberg.table.locations) - Catalog import failures (pyiceberg.catalog.__init__) - File deletion failures (pyiceberg.catalog.__init__) Users now see clean one-line warnings instead of full tracebacks. Developers can enable debug logging to see detailed exception info. --- pyiceberg/catalog/__init__.py | 9 ++++++--- pyiceberg/io/__init__.py | 3 ++- pyiceberg/table/locations.py | 3 ++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/pyiceberg/catalog/__init__.py b/pyiceberg/catalog/__init__.py index a4f1d47bea..9dfa6e2aaa 100644 --- a/pyiceberg/catalog/__init__.py +++ b/pyiceberg/catalog/__init__.py @@ -286,7 +286,8 @@ def delete_files(io: FileIO, files_to_delete: set[str], file_type: str) -> None: try: io.delete(file) except OSError as exc: - logger.warning(msg=f"Failed to delete {file_type} file {file}", exc_info=exc) + logger.warning(msg=f"Failed to delete {file_type} file {file}") + logger.debug(f"Error deleting {file_type} file {file}", exc_info=exc) def delete_data_files(io: FileIO, manifests_to_delete: list[ManifestFile]) -> None: @@ -306,7 +307,8 @@ def delete_data_files(io: FileIO, manifests_to_delete: list[ManifestFile]) -> No try: io.delete(path) except OSError as exc: - logger.warning(msg=f"Failed to delete data file {path}", exc_info=exc) + logger.warning(msg=f"Failed to delete data file {path}") + logger.debug(f"Error deleting data file {path}", exc_info=exc) deleted_files[path] = True @@ -320,7 +322,8 @@ def _import_catalog(name: str, catalog_impl: str, properties: Properties) -> Cat class_ = getattr(module, class_name) return class_(name, **properties) except ModuleNotFoundError as exc: - logger.warning(f"Could not initialize Catalog: {catalog_impl}", exc_info=exc) + logger.warning(f"Could not initialize Catalog: {catalog_impl}") + logger.debug(f"Failed to load {catalog_impl}", exc_info=exc) return None diff --git a/pyiceberg/io/__init__.py b/pyiceberg/io/__init__.py index c7109993c9..1cc046e893 100644 --- a/pyiceberg/io/__init__.py +++ b/pyiceberg/io/__init__.py @@ -322,7 +322,8 @@ def _import_file_io(io_impl: str, properties: Properties) -> FileIO | None: class_ = getattr(module, class_name) return class_(properties) except ModuleNotFoundError as exc: - logger.warning(f"Could not initialize FileIO: {io_impl}", exc_info=exc) + logger.warning(f"Could not initialize FileIO: {io_impl}") + logger.debug(f"Failed to load {io_impl}", exc_info=exc) return None diff --git a/pyiceberg/table/locations.py b/pyiceberg/table/locations.py index 25da7e7f6c..bcde1317ed 100644 --- a/pyiceberg/table/locations.py +++ b/pyiceberg/table/locations.py @@ -179,7 +179,8 @@ def _import_location_provider( class_ = getattr(module, class_name) return class_(table_location, table_properties) except ModuleNotFoundError as exc: - logger.warning(f"Could not initialize LocationProvider: {location_provider_impl}", exc_info=exc) + logger.warning(f"Could not initialize LocationProvider: {location_provider_impl}") + logger.debug(f"Failed to load {location_provider_impl}", exc_info=exc) return None From 13c7941ce3e6ac172804a3fb5b3d7ff62a1b1103 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sat, 27 Dec 2025 19:18:24 -0800 Subject: [PATCH 2/6] fix tests --- tests/io/test_io.py | 2 +- tests/table/test_locations.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/io/test_io.py b/tests/io/test_io.py index ac1d7b4fe4..3a2b5b5843 100644 --- a/tests/io/test_io.py +++ b/tests/io/test_io.py @@ -280,7 +280,7 @@ def test_import_file_io() -> None: def test_import_file_io_does_not_exist(caplog: Any) -> None: assert _import_file_io("pyiceberg.does.not.exist.FileIO", {}) is None - assert "ModuleNotFoundError: No module named 'pyiceberg.does'" in caplog.text + assert "Could not initialize FileIO: pyiceberg.does.not.exist.FileIO" in caplog.text def test_load_file() -> None: diff --git a/tests/table/test_locations.py b/tests/table/test_locations.py index 3634151de7..bbdaa91c17 100644 --- a/tests/table/test_locations.py +++ b/tests/table/test_locations.py @@ -70,7 +70,7 @@ def test_custom_location_provider_not_found(caplog: Any) -> None: load_location_provider( table_location="table_location", table_properties={"write.py-location-provider.impl": "module.not_found"} ) - assert "ModuleNotFoundError: No module named 'module'" in caplog.text + assert "Could not initialize LocationProvider: module.not_found" in caplog.text def test_object_storage_no_partition() -> None: From e9566405f6472d44b7f7d59541d0e19cb4743416 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 28 Dec 2025 12:12:57 -0800 Subject: [PATCH 3/6] thx drew --- pyiceberg/catalog/__init__.py | 9 +++------ pyiceberg/io/__init__.py | 3 +-- pyiceberg/table/locations.py | 3 +-- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/pyiceberg/catalog/__init__.py b/pyiceberg/catalog/__init__.py index 9dfa6e2aaa..167aa45f7d 100644 --- a/pyiceberg/catalog/__init__.py +++ b/pyiceberg/catalog/__init__.py @@ -286,8 +286,7 @@ def delete_files(io: FileIO, files_to_delete: set[str], file_type: str) -> None: try: io.delete(file) except OSError as exc: - logger.warning(msg=f"Failed to delete {file_type} file {file}") - logger.debug(f"Error deleting {file_type} file {file}", exc_info=exc) + logger.warning(f"Failed to delete {file_type} file {file}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None) def delete_data_files(io: FileIO, manifests_to_delete: list[ManifestFile]) -> None: @@ -307,8 +306,7 @@ def delete_data_files(io: FileIO, manifests_to_delete: list[ManifestFile]) -> No try: io.delete(path) except OSError as exc: - logger.warning(msg=f"Failed to delete data file {path}") - logger.debug(f"Error deleting data file {path}", exc_info=exc) + logger.warning(f"Failed to delete data file {path}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None) deleted_files[path] = True @@ -322,8 +320,7 @@ def _import_catalog(name: str, catalog_impl: str, properties: Properties) -> Cat class_ = getattr(module, class_name) return class_(name, **properties) except ModuleNotFoundError as exc: - logger.warning(f"Could not initialize Catalog: {catalog_impl}") - logger.debug(f"Failed to load {catalog_impl}", exc_info=exc) + logger.warning(f"Could not initialize Catalog: {catalog_impl}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None) return None diff --git a/pyiceberg/io/__init__.py b/pyiceberg/io/__init__.py index 1cc046e893..8e24e8090a 100644 --- a/pyiceberg/io/__init__.py +++ b/pyiceberg/io/__init__.py @@ -322,8 +322,7 @@ def _import_file_io(io_impl: str, properties: Properties) -> FileIO | None: class_ = getattr(module, class_name) return class_(properties) except ModuleNotFoundError as exc: - logger.warning(f"Could not initialize FileIO: {io_impl}") - logger.debug(f"Failed to load {io_impl}", exc_info=exc) + logger.warning(f"Could not initialize FileIO: {io_impl}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None) return None diff --git a/pyiceberg/table/locations.py b/pyiceberg/table/locations.py index bcde1317ed..a11d3b799e 100644 --- a/pyiceberg/table/locations.py +++ b/pyiceberg/table/locations.py @@ -179,8 +179,7 @@ def _import_location_provider( class_ = getattr(module, class_name) return class_(table_location, table_properties) except ModuleNotFoundError as exc: - logger.warning(f"Could not initialize LocationProvider: {location_provider_impl}") - logger.debug(f"Failed to load {location_provider_impl}", exc_info=exc) + logger.warning(f"Could not initialize LocationProvider: {location_provider_impl}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None) return None From 378e8d6d5097b0856c8c445700f8626590f2eefc Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 28 Dec 2025 12:15:32 -0800 Subject: [PATCH 4/6] make lint --- pyiceberg/catalog/__init__.py | 12 +++++++++--- pyiceberg/table/locations.py | 5 ++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/pyiceberg/catalog/__init__.py b/pyiceberg/catalog/__init__.py index 167aa45f7d..1a1043738d 100644 --- a/pyiceberg/catalog/__init__.py +++ b/pyiceberg/catalog/__init__.py @@ -286,7 +286,9 @@ def delete_files(io: FileIO, files_to_delete: set[str], file_type: str) -> None: try: io.delete(file) except OSError as exc: - logger.warning(f"Failed to delete {file_type} file {file}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None) + logger.warning( + f"Failed to delete {file_type} file {file}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None + ) def delete_data_files(io: FileIO, manifests_to_delete: list[ManifestFile]) -> None: @@ -306,7 +308,9 @@ def delete_data_files(io: FileIO, manifests_to_delete: list[ManifestFile]) -> No try: io.delete(path) except OSError as exc: - logger.warning(f"Failed to delete data file {path}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None) + logger.warning( + f"Failed to delete data file {path}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None + ) deleted_files[path] = True @@ -320,7 +324,9 @@ def _import_catalog(name: str, catalog_impl: str, properties: Properties) -> Cat class_ = getattr(module, class_name) return class_(name, **properties) except ModuleNotFoundError as exc: - logger.warning(f"Could not initialize Catalog: {catalog_impl}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None) + logger.warning( + f"Could not initialize Catalog: {catalog_impl}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None + ) return None diff --git a/pyiceberg/table/locations.py b/pyiceberg/table/locations.py index a11d3b799e..4d62fa3729 100644 --- a/pyiceberg/table/locations.py +++ b/pyiceberg/table/locations.py @@ -179,7 +179,10 @@ def _import_location_provider( class_ = getattr(module, class_name) return class_(table_location, table_properties) except ModuleNotFoundError as exc: - logger.warning(f"Could not initialize LocationProvider: {location_provider_impl}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None) + logger.warning( + f"Could not initialize LocationProvider: {location_provider_impl}", + exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None, + ) return None From f83ac214f12b6b871e21ca9e9d7b6633c848d21f Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 28 Dec 2025 12:15:34 -0800 Subject: [PATCH 5/6] fix tests --- tests/io/test_io.py | 4 ++++ tests/table/test_locations.py | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/tests/io/test_io.py b/tests/io/test_io.py index 3a2b5b5843..d9bee33f8b 100644 --- a/tests/io/test_io.py +++ b/tests/io/test_io.py @@ -279,8 +279,12 @@ def test_import_file_io() -> None: def test_import_file_io_does_not_exist(caplog: Any) -> None: + import logging + + caplog.set_level(logging.DEBUG) assert _import_file_io("pyiceberg.does.not.exist.FileIO", {}) is None assert "Could not initialize FileIO: pyiceberg.does.not.exist.FileIO" in caplog.text + assert "ModuleNotFoundError: No module named 'pyiceberg.does'" in caplog.text def test_load_file() -> None: diff --git a/tests/table/test_locations.py b/tests/table/test_locations.py index bbdaa91c17..6dd7520f3c 100644 --- a/tests/table/test_locations.py +++ b/tests/table/test_locations.py @@ -66,11 +66,15 @@ def test_custom_location_provider_single_path() -> None: def test_custom_location_provider_not_found(caplog: Any) -> None: + import logging + + caplog.set_level(logging.DEBUG) with pytest.raises(ValueError, match=r"Could not initialize LocationProvider"): load_location_provider( table_location="table_location", table_properties={"write.py-location-provider.impl": "module.not_found"} ) assert "Could not initialize LocationProvider: module.not_found" in caplog.text + assert "ModuleNotFoundError: No module named 'module'" in caplog.text def test_object_storage_no_partition() -> None: From 59c92cc9a2d732dc911939bbc5a49a9efb096024 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 28 Dec 2025 12:42:21 -0800 Subject: [PATCH 6/6] more simpified --- pyiceberg/catalog/__init__.py | 18 ++++++------------ pyiceberg/io/__init__.py | 4 ++-- pyiceberg/table/locations.py | 4 ++-- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/pyiceberg/catalog/__init__.py b/pyiceberg/catalog/__init__.py index 1a1043738d..d1d83c6dbf 100644 --- a/pyiceberg/catalog/__init__.py +++ b/pyiceberg/catalog/__init__.py @@ -285,10 +285,8 @@ def delete_files(io: FileIO, files_to_delete: set[str], file_type: str) -> None: for file in files_to_delete: try: io.delete(file) - except OSError as exc: - logger.warning( - f"Failed to delete {file_type} file {file}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None - ) + except OSError: + logger.warning(f"Failed to delete {file_type} file {file}", exc_info=logger.isEnabledFor(logging.DEBUG)) def delete_data_files(io: FileIO, manifests_to_delete: list[ManifestFile]) -> None: @@ -307,10 +305,8 @@ def delete_data_files(io: FileIO, manifests_to_delete: list[ManifestFile]) -> No if not deleted_files.get(path, False): try: io.delete(path) - except OSError as exc: - logger.warning( - f"Failed to delete data file {path}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None - ) + except OSError: + logger.warning(f"Failed to delete data file {path}", exc_info=logger.isEnabledFor(logging.DEBUG)) deleted_files[path] = True @@ -323,10 +319,8 @@ def _import_catalog(name: str, catalog_impl: str, properties: Properties) -> Cat module = importlib.import_module(module_name) class_ = getattr(module, class_name) return class_(name, **properties) - except ModuleNotFoundError as exc: - logger.warning( - f"Could not initialize Catalog: {catalog_impl}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None - ) + except ModuleNotFoundError: + logger.warning(f"Could not initialize Catalog: {catalog_impl}", exc_info=logger.isEnabledFor(logging.DEBUG)) return None diff --git a/pyiceberg/io/__init__.py b/pyiceberg/io/__init__.py index 8e24e8090a..87d155a0fd 100644 --- a/pyiceberg/io/__init__.py +++ b/pyiceberg/io/__init__.py @@ -321,8 +321,8 @@ def _import_file_io(io_impl: str, properties: Properties) -> FileIO | None: module = importlib.import_module(module_name) class_ = getattr(module, class_name) return class_(properties) - except ModuleNotFoundError as exc: - logger.warning(f"Could not initialize FileIO: {io_impl}", exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None) + except ModuleNotFoundError: + logger.warning(f"Could not initialize FileIO: {io_impl}", exc_info=logger.isEnabledFor(logging.DEBUG)) return None diff --git a/pyiceberg/table/locations.py b/pyiceberg/table/locations.py index 4d62fa3729..3aeaaf228b 100644 --- a/pyiceberg/table/locations.py +++ b/pyiceberg/table/locations.py @@ -178,10 +178,10 @@ def _import_location_provider( module = importlib.import_module(module_name) class_ = getattr(module, class_name) return class_(table_location, table_properties) - except ModuleNotFoundError as exc: + except ModuleNotFoundError: logger.warning( f"Could not initialize LocationProvider: {location_provider_impl}", - exc_info=exc if logger.isEnabledFor(logging.DEBUG) else None, + exc_info=logger.isEnabledFor(logging.DEBUG), ) return None