Skip to content

Commit 546bd46

Browse files
fix: Implement two-tier exception handling for Calamine panics and update test
- Add two-tier exception handling: catch Exception first, then BaseException - PyO3 PanicException from Calamine inherits from BaseException, not Exception - Keep targeted BLE001 suppression with explanatory comment - Re-raise KeyboardInterrupt/SystemExit in BaseException handler - Update calamine_exc type to Optional[BaseException] for MyPy - Update test mocks to accept sheet_name parameter - Verified: test passes and MyPy succeeds locally This preserves the functional requirement to catch Calamine panics while following Python best practices for normal exception handling. Co-Authored-By: unknown <>
1 parent 88084ad commit 546bd46

File tree

2 files changed

+19
-3
lines changed

2 files changed

+19
-3
lines changed

airbyte_cdk/sources/file_based/file_types/excel_parser.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ def open_and_parse_file(
200200
file_url = getattr(file_info, "url", None)
201201
elif isinstance(file_info, str):
202202
file_label = file_info
203-
calamine_exc: Optional[Exception] = None
203+
calamine_exc: Optional[BaseException] = None
204204
try:
205205
with pd.ExcelFile(fp, engine="calamine") as excel_file: # type: ignore [arg-type, call-overload]
206206
return excel_file.parse(sheet_name=0) # type: ignore [no-any-return]
@@ -213,6 +213,18 @@ def open_and_parse_file(
213213
file_url,
214214
)
215215
)
216+
except BaseException as exc: # noqa: BLE001
217+
# PyO3 PanicException from Calamine inherits from BaseException, not Exception
218+
if isinstance(exc, (KeyboardInterrupt, SystemExit)):
219+
raise
220+
calamine_exc = exc
221+
if logger:
222+
logger.warning(
223+
ExcelParser._format_message_with_link(
224+
f"Calamine parsing failed for {file_label}, falling back to openpyxl: {exc}",
225+
file_url,
226+
)
227+
)
216228

217229
# Fallback to openpyxl
218230
try:

unit_tests/sources/file_based/file_types/test_excel_parser.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,17 @@ def test_open_and_parse_file_falls_back_to_openpyxl(mock_logger):
152152
calamine_ctx = MagicMock()
153153
calamine_excel_file = MagicMock()
154154
calamine_ctx.__enter__.return_value = calamine_excel_file
155-
calamine_excel_file.parse.side_effect = FakePanic("calamine panic")
155+
156+
def calamine_parse_side_effect(sheet_name=None):
157+
raise FakePanic("calamine panic")
158+
159+
calamine_excel_file.parse.side_effect = calamine_parse_side_effect
156160

157161
openpyxl_ctx = MagicMock()
158162
openpyxl_excel_file = MagicMock()
159163
openpyxl_ctx.__enter__.return_value = openpyxl_excel_file
160164

161-
def openpyxl_parse_side_effect():
165+
def openpyxl_parse_side_effect(sheet_name=None):
162166
warnings.warn("Cell A146 has invalid date", UserWarning)
163167
return fallback_df
164168

0 commit comments

Comments
 (0)