Skip to content

Conversation

@27Bslash6
Copy link
Contributor

Summary

Fixes a timing mismatch in the import guard that caused HAS_ARROW_SERIALIZER to be incorrectly set to True even when pyarrow wasn't installed.

The Bug

# auto_serializer.py
try:
    from .arrow_serializer import ArrowSerializer
    HAS_ARROW_SERIALIZER = True  # ← This succeeded!
except ImportError:
    HAS_ARROW_SERIALIZER = False

# arrow_serializer.py  
try:
    import pyarrow as pa
    HAS_PYARROW = True
except ImportError:
    HAS_PYARROW = False  # ← pyarrow missing, but class still defined

class ArrowSerializer:  # ← Class is importable even without pyarrow!
    def __init__(self):
        if not HAS_PYARROW:
            raise ImportError(...)  # ← Too late - guard already passed

The Fix

Fail-fast at module level so the import guard catches it:

# arrow_serializer.py
try:
    import pyarrow as pa
except ImportError as e:
    raise ImportError("pyarrow is not installed...") from e  # Fail here, not in __init__

Test plan

  • make quick-check passes
  • All pre-commit hooks pass

@codecov
Copy link

codecov bot commented Dec 17, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/cachekit/serializers/arrow_serializer.py 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

The import guard in auto_serializer.py was ineffective because
ArrowSerializer class was always defined and importable - the
ImportError only fired in __init__(), after the guard had passed.

Fix: Raise ImportError at module level when pyarrow isn't installed,
so the import guard actually catches it:

Before (broken):
  auto_serializer.py: from .arrow_serializer import ArrowSerializer ✓
  auto_serializer.py: HAS_ARROW_SERIALIZER = True  # Wrong!
  ArrowSerializer.__init__(): raise ImportError  # Too late

After (fixed):
  arrow_serializer.py: import pyarrow → ImportError
  auto_serializer.py: from .arrow_serializer... → ImportError caught
  auto_serializer.py: HAS_ARROW_SERIALIZER = False  # Correct!

Adds test coverage for the import guard by mocking pyarrow unavailability.
@27Bslash6 27Bslash6 force-pushed the fix/arrow-serializer-import-guard branch from c460108 to 7dd8597 Compare December 17, 2025 23:47
@27Bslash6 27Bslash6 merged commit e6ab19e into main Dec 18, 2025
36 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants