Skip to content

Commit 7b84d10

Browse files
authored
Clean up logging: move exception tracebacks to debug level (apache#2867)
<!-- Thanks for opening a pull request! --> <!-- In the case this PR will resolve an issue, please replace ${GITHUB_ISSUE_ID} below with the actual Github issue id. --> <!-- Closes #${GITHUB_ISSUE_ID} --> # Rationale for this change I noticed we dump the entire error traceback in log warn messages. This causes a wall of text when using the CLI. This PR conditionally dump traceback only for `debug` log level. This provides cleaner user-facing output by default while still making detailed exception info available for troubleshooting via debug logging. - Users now see clean one-line warnings instead of full tracebacks. - Developers can enable debug logging to see detailed exception info. Using the shorthand, `exc_info` in logging.warning's kwargs: https://docs.python.org/3/library/logging.html#:~:text=If%20exc_info%20does%20not%20evaluate%20as%20false%2C%20it%20causes%20exception%20information%20to%20be%20added%20to%20the%20logging%20message.%20If%20an%20exception%20tuple%20(in%20the%20format%20returned%20by%20sys.exc_info())%20or%20an%20exception%20instance%20is%20provided%2C%20it%20is%20used%3B%20otherwise%2C%20sys.exc_info()%20is%20called%20to%20get%20the%20exception%20information. ## Are these changes tested? ## Are there any user-facing changes? <!-- In the case of user-facing changes, please add the changelog label. --> #### Before ``` Could not initialize FileIO: pyiceberg.io.pyarrow.PyArrowFileIO Traceback (most recent call last): File "/Users/kevinliu/repos/iceberg-python/pyiceberg/io/__init__.py", line 321, in _import_file_io module = importlib.import_module(module_name) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/kevinliu/.pyenv/versions/3.12.11/lib/python3.12/importlib/__init__.py", line 90, in import_module return _bootstrap._gcd_import(name[level:], package, level) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "<frozen importlib._bootstrap>", line 1387, in _gcd_import File "<frozen importlib._bootstrap>", line 1360, in _find_and_load File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked File "<frozen importlib._bootstrap>", line 935, in _load_unlocked File "<frozen importlib._bootstrap_external>", line 999, in exec_module File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed File "/Users/kevinliu/repos/iceberg-python/pyiceberg/io/pyarrow.py", line 54, in <module> import pyarrow as pa ModuleNotFoundError: No module named 'pyarrow' ``` #### After ``` Could not initialize FileIO: pyiceberg.io.pyarrow.PyArrowFileIO ```
1 parent 8aa8515 commit 7b84d10

5 files changed

Lines changed: 21 additions & 10 deletions

File tree

pyiceberg/catalog/__init__.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,8 @@ def delete_files(io: FileIO, files_to_delete: set[str], file_type: str) -> None:
285285
for file in files_to_delete:
286286
try:
287287
io.delete(file)
288-
except OSError as exc:
289-
logger.warning(msg=f"Failed to delete {file_type} file {file}", exc_info=exc)
288+
except OSError:
289+
logger.warning(f"Failed to delete {file_type} file {file}", exc_info=logger.isEnabledFor(logging.DEBUG))
290290

291291

292292
def delete_data_files(io: FileIO, manifests_to_delete: list[ManifestFile]) -> None:
@@ -305,8 +305,8 @@ def delete_data_files(io: FileIO, manifests_to_delete: list[ManifestFile]) -> No
305305
if not deleted_files.get(path, False):
306306
try:
307307
io.delete(path)
308-
except OSError as exc:
309-
logger.warning(msg=f"Failed to delete data file {path}", exc_info=exc)
308+
except OSError:
309+
logger.warning(f"Failed to delete data file {path}", exc_info=logger.isEnabledFor(logging.DEBUG))
310310
deleted_files[path] = True
311311

312312

@@ -319,8 +319,8 @@ def _import_catalog(name: str, catalog_impl: str, properties: Properties) -> Cat
319319
module = importlib.import_module(module_name)
320320
class_ = getattr(module, class_name)
321321
return class_(name, **properties)
322-
except ModuleNotFoundError as exc:
323-
logger.warning(f"Could not initialize Catalog: {catalog_impl}", exc_info=exc)
322+
except ModuleNotFoundError:
323+
logger.warning(f"Could not initialize Catalog: {catalog_impl}", exc_info=logger.isEnabledFor(logging.DEBUG))
324324
return None
325325

326326

pyiceberg/io/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,8 +321,8 @@ def _import_file_io(io_impl: str, properties: Properties) -> FileIO | None:
321321
module = importlib.import_module(module_name)
322322
class_ = getattr(module, class_name)
323323
return class_(properties)
324-
except ModuleNotFoundError as exc:
325-
logger.warning(f"Could not initialize FileIO: {io_impl}", exc_info=exc)
324+
except ModuleNotFoundError:
325+
logger.warning(f"Could not initialize FileIO: {io_impl}", exc_info=logger.isEnabledFor(logging.DEBUG))
326326
return None
327327

328328

pyiceberg/table/locations.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,11 @@ def _import_location_provider(
178178
module = importlib.import_module(module_name)
179179
class_ = getattr(module, class_name)
180180
return class_(table_location, table_properties)
181-
except ModuleNotFoundError as exc:
182-
logger.warning(f"Could not initialize LocationProvider: {location_provider_impl}", exc_info=exc)
181+
except ModuleNotFoundError:
182+
logger.warning(
183+
f"Could not initialize LocationProvider: {location_provider_impl}",
184+
exc_info=logger.isEnabledFor(logging.DEBUG),
185+
)
183186
return None
184187

185188

tests/io/test_io.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,11 @@ def test_import_file_io() -> None:
279279

280280

281281
def test_import_file_io_does_not_exist(caplog: Any) -> None:
282+
import logging
283+
284+
caplog.set_level(logging.DEBUG)
282285
assert _import_file_io("pyiceberg.does.not.exist.FileIO", {}) is None
286+
assert "Could not initialize FileIO: pyiceberg.does.not.exist.FileIO" in caplog.text
283287
assert "ModuleNotFoundError: No module named 'pyiceberg.does'" in caplog.text
284288

285289

tests/table/test_locations.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,14 @@ def test_custom_location_provider_single_path() -> None:
6666

6767

6868
def test_custom_location_provider_not_found(caplog: Any) -> None:
69+
import logging
70+
71+
caplog.set_level(logging.DEBUG)
6972
with pytest.raises(ValueError, match=r"Could not initialize LocationProvider"):
7073
load_location_provider(
7174
table_location="table_location", table_properties={"write.py-location-provider.impl": "module.not_found"}
7275
)
76+
assert "Could not initialize LocationProvider: module.not_found" in caplog.text
7377
assert "ModuleNotFoundError: No module named 'module'" in caplog.text
7478

7579

0 commit comments

Comments
 (0)