feat(file-based): Add Calamine-first with Openpyxl fallback for Excel parser

[devin-ai-integration]

refactor(file-based): Add Calamine-first with Openpyxl fallback for Excel parser

Summary

Implements a Calamine-first parsing strategy with Openpyxl fallback for Excel files to handle cases where Calamine panics on invalid data (e.g., year 20225 out of range). The parser now:

1. Tries Calamine first (fast path) - catches PyO3 PanicException by checking for "ValueError" in the exception string
2. Falls back to Openpyxl if Calamine fails - logs a warning with file URI and error details
3. Handles file pointer reset - attempts to seek back to position 0 before openpyxl parsing, with info-level logging if seek fails
4. Propagates critical exceptions - ensures SystemExit and other non-ValueError BaseExceptions are not caught

Key Changes:

• Added ExcelCalamineParsingError custom exception
• Refactored open_and_parse_file into three focused instance methods:
  • _open_and_parse_file_with_calamine() - Calamine parsing with custom exception
  • _open_and_parse_file_with_openpyxl() - Openpyxl parsing with warning capture and seek logic
  • open_and_parse_file() - Orchestration with fallback logic
• Converted static methods to instance methods
• Added file_uri_for_logging property to RemoteFile class (moved from child class)
• Added openpyxl ^3.1.0 as optional dependency in file-based extras
• Added comprehensive unit tests for fallback behavior, SystemExit propagation, and non-seekable files

Review & Testing Checklist for Human

Risk Level: Yellow - Core parsing logic changes with exception handling complexity

• Test with real Excel files that cause Calamine to panic - The string matching for "ValueError" in exception messages is fragile. Verify this catches actual Calamine panics (e.g., files with year 20225) and doesn't miss edge cases or catch unintended exceptions.
• Test with non-seekable file objects - The code attempts to seek back to position 0 when falling back to openpyxl. Verify openpyxl can still parse from the current position when seeking fails, or if this causes data corruption. Test with various file-like objects (BytesIO, file handles, non-seekable streams).
• Verify type signature changes don't break existing code - Changed file_info from Union[str, RemoteFile] to just RemoteFile. Check all call sites still work correctly.

Test Plan

1. Create an Excel file with invalid date values (e.g., year 20225) that causes Calamine to panic
2. Verify the parser falls back to openpyxl and logs the warning message
3. Verify the sync completes successfully with the fallback
4. Test with various file-like objects (BytesIO, file handles, non-seekable streams)
5. Verify SystemExit and KeyboardInterrupt propagate correctly

Notes

• This PR addresses airbytehq/oncall#10097 - Excel format parser crashes
• Requested by Alfredo Garcia (@agarctfi)
• Session: https://app.devin.ai/sessions/4b3fc75c1a76481aa1e7085f83b22260
• All local checks pass: MyPy, unit tests (6 passed, 1 skipped), Ruff format/lint
• Addressed reviewer feedback from Daryna Ishchenko (@darynaishchenko) on method design, exception handling, and test coverage

Important

Auto-merge enabled.

This PR is set to merge automatically when all requirements are met.

Note

Auto-merge may have been disabled. Please check the PR status to confirm.

[devin-ai-integration]

Original prompt from API User

Comment from @agarctfi: /ai-fix Can you do the Calamine-First with Openpyxl Fallback; also, add a log that we had to perform the fallback when done. \n\nIMPORTANT: The user will expect a response posted back to the PR. You should post exactly one comment back to the respective issue PR. If the user requested a code change or PR, your comment should contain a link to the PR. Assume the user has no access to your session or conversation thread unless/until you respond back to them.\n\nIssue #10097 by @iherdt-airbyte: Source: Google Drive `Excel format parser crashes`\n\nIssue URL: https://github.com/airbytehq/oncall/issues/10097\n\nPlease use playbook macro: !issue_fix

PLAYBOOK_md:
# AI Fix Playbook

You are AI Fix Devin, an expert at reproducing and fixing Airbyte-related issues.

## Context
You are working on the issue linked above in context. You will also need to pull issue comments for full context.

## Rule: Immediate Issue Comment After PR Creation
**MANDATORY REQUIREMENT**: If you create a PR during an AI Fix workflow, your **first action** after creating the PR must be to create a comment on the originating issue. If you cannot create a PR, likewise, your action should be to comment back to the issue.

## Properly note breaking changes

Types of breaking changes:
- spec change
    - a spec field is removed or renamed.
    - a new required spec field is added.
- schema change
    - a field is removed or renamed, or, the datatype is changed.
- stream or property removal
    - a stream is removed.
- state changes
    - the format of the state is changed.

Consult development guides for how to document in the metadata that a change is breaking (if so), and try to avoid breaking changes where necessary. Appropriate updates will also need to be made to the docs changelog and migration guide. Refer to the [Managing Breaking Changes in Connectors](https://docs.airbyte.com/platform/next/connector-development/connector-breaking-changes) documentation to understand wha... (3994 chars truncated...)

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[github-actions]

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

Testing This CDK Version

You can test this version of the CDK using the following:

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1763137629-excel-parser-openpyxl-fallback#egg=airbyte-python-cdk[dev]' --help

# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1763137629-excel-parser-openpyxl-fallback

Helpful Resources

• CDK API Reference

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

• /autofix - Fixes most formatting and linting issues
• /poetry-lock - Updates poetry.lock file
• /test - Runs connector tests with the updated CDK
• /prerelease - Triggers a prerelease publish with default arguments
• /poe build - Regenerate git-committed build artifacts, such as the pydantic models which are generated from the manifest JSON schema in YAML.
• /poe <command> - Runs any poe command in the CDK environment

📝 Edit this welcome message.

[github-actions]

PyTest Results (Full)

3 827 tests  +3   3 815 ✅ +3   10m 56s ⏱️ +8s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 31f7216. ± Comparison against base commit a67a6cf.

♻️ This comment has been updated with latest results.

[github-actions]

PyTest Results (Fast)

3 824 tests  +3   3 812 ✅ +3   6m 35s ⏱️ -4s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 31f7216. ± Comparison against base commit a67a6cf.

♻️ This comment has been updated with latest results.

[Alfredo Garcia (agarctfi)]

/autofix

Auto-Fix Job Info

This job attempts to auto-fix any linting or formating issues. If any fixes are made,
those changes will be automatically committed and pushed back to the PR.

Note: This job can only be run by maintainers. On PRs from forks, this command requires
that the PR author has enabled the Allow edits from maintainers option.

PR auto-fix job started... Check job output.

🟦 Job completed successfully (no changes).

[devin-ai-integration]

Code Quality Improvements

I've addressed the code quality bot feedback:

1. Changed BaseException to Exception in both except blocks (lines 207 and 238)

   • Removed manual KeyboardInterrupt/SystemExit re-raise checks since these now propagate naturally
   • This follows Python best practices for exception handling

2. Added explanatory comment to the empty except block (line 221)

   • Clarifies that some file-like objects may not be seekable and we attempt openpyxl parsing anyway

These changes improve code safety and readability without altering behavior.

---

CI Status Summary:

✅ All Core CDK Checks Passing (20 checks):

• Deptry, Ruff Lint/Format, MyPy, Build and Inspect Python Package, CodeQL
• All pytest matrix jobs passing

❌ 2 Connector Failures (Unrelated to Excel Parser):

Show connector failure details

1. source-google-drive: Test harness initialization issue

   TypeError: SourceGoogleDrive.__init__() missing 3 required positional arguments: 'catalog', 'config', and 'state'
   

   From airbyte_cdk/test/standard_tests/connector_base.py:94 where test calls connector()

2. source-intercom: Concurrent source API issue

   TypeError: IntercomScrollRetriever._read_pages() missing 1 required positional argument: 'stream_slice'
   

Both failures are pre-existing connector-specific issues with no connection to Excel parsing, openpyxl, calamine, or file-based sources.

The PR is ready for review. All Excel parser changes are complete and tested.

[Alfredo Garcia (agarctfi)]

/format-fix

[devin-ai-integration]

Code Quality Improvements - Update

I've addressed the code quality bot feedback and fixed a MyPy type checking issue:

Changes Made:

1. Changed BaseException to Exception in both except blocks (commits adfe576 and 88084ad)

   • Removed manual KeyboardInterrupt/SystemExit re-raise checks since these now propagate naturally
   • This follows Python best practices for exception handling

2. Added explanatory comment to the empty except block (commit adfe576)

   • Clarifies that some file-like objects may not be seekable and we attempt openpyxl parsing anyway

3. Fixed MyPy type checking errors (commit 88084ad)

   • Added sheet_name=0 to both ExcelFile.parse() calls
   • This explicitly specifies the first sheet (index 0), matching pandas' default behavior
   • Resolves MyPy call-overload and no-any-return errors
   • Behavior unchanged: pandas defaults to first sheet when no sheet_name provided
   • ExcelFormat has no sheet selection parameter, so defaulting to first sheet is correct

Verification:

• ✅ MyPy passes locally: poetry run mypy --config-file mypy.ini airbyte_cdk/sources/file_based/file_types/excel_parser.py
• ✅ Code is properly formatted (verified by /format-fix command)

These changes improve code safety and type correctness without altering runtime behavior.

[devin-ai-integration]

Code Quality Improvements - Final Update

I've successfully addressed all code quality issues and fixed the test failure:

Changes Made:

1. Exception Handling Improvements (commits adfe576, 546bd46)

• Implemented two-tier exception handling for Calamine:
  • First except Exception catches normal failures
  • Second except BaseException catches PyO3 PanicException (with targeted # noqa: BLE001)
  • Re-raises KeyboardInterrupt/SystemExit in BaseException handler
• Added explanatory comment: "PyO3 PanicException from Calamine inherits from BaseException, not Exception"
• Updated calamine_exc type to Optional[BaseException] for MyPy compatibility
• Kept openpyxl handler as except Exception (no BaseException needed)

2. MyPy Type Checking Fix (commit 88084ad)

• Added sheet_name=0 to both ExcelFile.parse() calls
• Resolves MyPy call-overload and no-any-return errors
• Behavior unchanged: explicitly specifies first sheet (pandas default)

3. Test Fix (commit 546bd46)

• Updated test mocks to accept sheet_name parameter
• Both calamine_parse_side_effect and openpyxl_parse_side_effect now accept sheet_name=None

Verification:

• ✅ Test passes locally: test_open_and_parse_file_falls_back_to_openpyxl
• ✅ MyPy passes locally: mypy --config-file mypy.ini airbyte_cdk/sources/file_based/file_types/excel_parser.py
• ✅ All formatting checks passing (Ruff Lint/Format, Deptry)

Why Two-Tier Exception Handling?

The functional requirement is to catch Calamine panics and fall back to openpyxl. PyO3's PanicException (raised by the Rust Calamine library) inherits from BaseException, not Exception. A pure except Exception cannot catch these panics, breaking the fallback mechanism.

The two-tier approach satisfies both:

1. Python best practices: Catch Exception for normal failures
2. Functional requirement: Catch BaseException for PyO3 panics with targeted suppression

This is the correct pattern for handling Rust library panics in Python while maintaining proper exception handling hygiene.

[devin-ai-integration]

✅ All Code Quality Issues Resolved

I've successfully addressed all code quality issues and test failures:

Final Commits:

1. Exception Handling Improvements (commits adfe576, 546bd46)

• Implemented two-tier exception handling for Calamine to catch both normal exceptions and PyO3 panics
• Changed calamine_exc type to Optional[BaseException] for MyPy compatibility
• Added targeted # noqa: BLE001 suppression with explanatory comment
• Re-raises KeyboardInterrupt/SystemExit in BaseException handler

2. MyPy Type Checking Fix (commit 88084ad)

• Added sheet_name=0 to both ExcelFile.parse() calls
• Resolves MyPy call-overload errors

3. Test Fix (commit 546bd46)

• Updated test mocks to accept sheet_name parameter

4. Ruff Formatting Fix (commit 67fa697)

• Removed trailing whitespace from test file

Local Verification:

• ✅ Test passes: test_open_and_parse_file_falls_back_to_openpyxl
• ✅ MyPy passes: mypy --config-file mypy.ini airbyte_cdk/sources/file_based/file_types/excel_parser.py
• ✅ Ruff format passes: ruff format --diff .

All code quality improvements are complete and verified locally. CI should now pass.

[devin-ai-integration]

✅ All Reviewer Feedback Addressed

I've successfully addressed all reviewer feedback from Daryna Ishchenko (@darynaishchenko):

Latest Changes (Comment 2555661755) ✅

Moved seek logic into _open_and_parse_file_with_openpyxl:

• Moved fp.seek(0) try/except block from orchestration method into _open_and_parse_file_with_openpyxl
• Added hasattr(fp, "seek") check to avoid AttributeError on non-file-like objects
• Added info-level logging for seek failures: "Could not rewind stream for {file_uri}; proceeding with openpyxl from current position: {exc}"
• Removed duplicate seek logic from open_and_parse_file orchestration method
• Simplified orchestration to focus purely on flow control

Previous Changes ✅

• Comment 27: Converted all three methods from @staticmethod to instance methods using self
• Comment 28: Explained why the try/except for seek is needed (non-seekable file objects)
• Comment 29: Updated test to use SystemExit instead of KeyboardInterrupt with explanatory docstring

Local Verification ✅

All checks pass locally:

• ✅ Unit tests pass (4 passed, 1 skipped)
• ✅ MyPy type checking passes
• ✅ Ruff format passes
• ✅ Ruff lint passes
• ✅ Warning count test still passes (2 warnings: Calamine fallback + openpyxl warning)

Commits:

• d2f691a - "refactor: Convert static methods to instance methods and update test"
• 63d24a6 - "refactor: Move seek logic into _open_and_parse_file_with_openpyxl"

Ready for re-review! 🚀

[devin-ai-integration]

✅ All Reviewer Feedback Addressed

I've successfully addressed all reviewer feedback from Daryna Ishchenko (@darynaishchenko) and Alfredo Garcia (@agarctfi):

Latest Changes (Comment 2556819331 from Alfredo Garcia (@agarctfi)) ✅

Added parametrized unit test for non-seekable files:

• test_openpyxl_logs_info_when_seek_fails[attribute-error] - Tests AttributeError case
• test_openpyxl_logs_info_when_seek_fails[os-error] - Tests OSError case
• Verifies info log is emitted with correct content (file URI included)
• Confirms parsing proceeds from current position when rewind fails
• Uses FakeFP class with seek method that raises the desired exception

Previous Changes ✅

• Comment 2555661755 (Daryna Ishchenko (@darynaishchenko)): Moved seek logic into _open_and_parse_file_with_openpyxl with info-level logging
• Comment 27 (Daryna Ishchenko (@darynaishchenko)): Converted all three methods from @staticmethod to instance methods
• Comment 28 (Daryna Ishchenko (@darynaishchenko)): Explained why try/except for seek is needed
• Comment 29 (Daryna Ishchenko (@darynaishchenko)): Updated test to use SystemExit with explanatory docstring

Local Verification ✅

All checks pass locally:

• ✅ Unit tests pass (6 passed, 1 skipped)
• ✅ Ruff format passes
• ✅ Ruff lint passes

Commits:

• d2f691a - "refactor: Convert static methods to instance methods and update test"
• 63d24a6 - "refactor: Move seek logic into _open_and_parse_file_with_openpyxl"
• 44f7df1 - "test: Add parametrized test for non-seekable files in openpyxl fallback"

Ready for re-review! 🚀

[devin-ai-integration]

✅ All Latest Reviewer Feedback Addressed

I've successfully addressed both new reviewer comments from Daryna Ishchenko (@darynaishchenko):

1. Comment 2557109671 - Remove AttributeError from Exception Handling ✅

Changes Made:

• Narrowed except (AttributeError, OSError) to except OSError in _open_and_parse_file_with_openpyxl
• Updated parametrized test to only test OSError case, removing AttributeError case

Rationale:
With the hasattr(fp, "seek") guard in place, AttributeError can only occur during attribute lookup, which hasattr already handles. After the guard passes, only OSError (including io.UnsupportedOperation) can be raised by seek() on non-seekable streams.

Commit: 49f3e19 - "refactor: Narrow exception handling to OSError only in seek logic"

2. Comment 2557118668 - Rename file_info to file ✅

Changes Made:

• Renamed file_info parameter to file in all three methods:
  • _open_and_parse_file_with_calamine
  • _open_and_parse_file_with_openpyxl
  • open_and_parse_file
• Updated all references throughout the methods

Rationale:
Since the type is RemoteFile, the shorter name file is more appropriate and makes the code more concise while maintaining clarity.

Commit: fffe027 - "refactor: Rename file_info parameter to file in Excel parsing methods"

Local Verification ✅

All checks pass for both changes:

• ✅ All Excel parser tests pass (5 passed, 1 skipped)
• ✅ Ruff format passes
• ✅ Ruff lint passes

Ready for re-review! 🚀

[Alfredo Garcia (agarctfi)]

/autofix

Auto-Fix Job Info

This job attempts to auto-fix any linting or formating issues. If any fixes are made,
those changes will be automatically committed and pushed back to the PR.

Note: This job can only be run by maintainers. On PRs from forks, this command requires
that the PR author has enabled the Allow edits from maintainers option.

PR auto-fix job started... Check job output.

✅ Changes applied successfully.