Log exception when FileIO import fails #1578
Conversation
There was a problem hiding this comment.
Thanks for the contribution!
we use the same pattern for loading custom location provider and custom catalog, do you mind also making the same changes there?
pyiceberg/io/__init__.py
Outdated
| except ModuleNotFoundError: | ||
| logger.warning("Could not initialize FileIO: %s", io_impl) |
There was a problem hiding this comment.
nit: what about something like this
iceberg-python/pyiceberg/catalog/__init__.py
Lines 287 to 288 in 1e01101
| @@ -281,6 +282,11 @@ def test_import_file_io_does_not_exist() -> None: | |||
| assert _import_file_io("pyiceberg.does.not.exist.FileIO", {}) is None | |||
|
|
|||
|
|
|||
| def test_import_file_io_logs_exception(caplog: Any) -> None: | |||
| _import_file_io("pyiceberg.does.not.exist.FileIO", {}) | |||
| assert "ModuleNotFoundError: No module named 'pyiceberg.does'" in caplog.text | |||
There was a problem hiding this comment.
does the error just return pyiceberg.does or the entire import, pyiceberg.does.not.exist.FileIO
There was a problem hiding this comment.
It fails here on pyiceberg.does. I presume that's Python trying and failing to import higher-level modules first?
There was a problem hiding this comment.
thanks! i was curious about the behavior
There was a problem hiding this comment.
Yeah, was also surprised
|
Updated the warnings for location providers and custom catalogs. |
| @@ -281,6 +282,11 @@ def test_import_file_io_does_not_exist() -> None: | |||
| assert _import_file_io("pyiceberg.does.not.exist.FileIO", {}) is None | |||
|
|
|||
|
|
|||
| def test_import_file_io_logs_exception(caplog: Any) -> None: | |||
| _import_file_io("pyiceberg.does.not.exist.FileIO", {}) | |||
| assert "ModuleNotFoundError: No module named 'pyiceberg.does'" in caplog.text | |||
There was a problem hiding this comment.
thanks! i was curious about the behavior
Closes #1577.
Log the underlying exception when a FileIO import fails.