feat(logging): add log_path parameter to setup_logger for file output

[cckellogg]

Changes Made

This adds an option to write logs to a file

[greptile-apps]

Greptile Summary

This PR adds an optional log_path parameter to setup_logger, allowing Daft log output to be mirrored to a file in addition to (or instead of) stream handlers. A _DaftFileHandler marker subclass is introduced so that subsequent calls to setup_logger can cleanly remove and replace any previously-installed file handler without disturbing user-created handlers.

Key changes:

• _DaftFileHandler(logging.FileHandler) — zero-behavior marker class used to identify Daft-managed file handlers for removal on re-configuration.
• The handler loop is now correctly iterated over a snapshot (list(root_logger.handlers)) to avoid mutation-during-iteration.
• Path handling uses Path.expanduser().resolve() plus path.parent.mkdir(parents=True, exist_ok=True), so nested log directories are created automatically.
• The log_path type annotation accepts str | Path | None, and the file handler correctly inherits the same level, formatter, and any daft_only/exclude_prefix filters as the existing stream handlers.
• The pre-existing import warnings statement inside setup_debug_logger() is inline inside a function, violating the project convention that all imports belong at the top of the file.

Confidence Score: 4/5

• This PR is safe to merge; the new file-logging logic is well-scoped, and previously raised concerns have been addressed.
• The implementation is clean and correct: handler lifecycle is managed with a marker class, the iteration snapshot fixes the previous mutation bug, path validation and directory creation are in place, and filters are properly propagated to the new file handler. The only remaining note is a pre-existing inline import that violates the project style rule but does not affect runtime behaviour.
• No files require special attention.

Important Files Changed

Filename	Overview
daft/logging.py	Adds log_path parameter to setup_logger, creates _DaftFileHandler marker class for lifecycle management, and correctly handles path expansion, parent directory creation, and filter re-application on repeated calls. One pre-existing inline import violates the project's import placement convention.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["setup_logger(level, log_path, ...)"] --> B["Validate level & normalise to upper-case"]
    B --> C["logging.basicConfig / root_logger.setLevel"]
    C --> D["Iterate snapshot of root_logger.handlers"]
    D --> E{"handler is\n_DaftFileHandler?"}
    E -- Yes --> F["removeHandler + handler.close()"]
    E -- No --> G["handler.filters.clear()"]
    F --> H{"log_path\nis not None?"}
    G --> H
    H -- Yes --> I["Path.expanduser().resolve()"]
    I --> J["Validate path.name is non-empty"]
    J --> K["path.parent.mkdir(parents=True)"]
    K --> L["Create _DaftFileHandler(path)"]
    L --> M["Set level, formatter → addHandler"]
    H -- No --> N["Skip file handler"]
    M --> O{"daft_only?"}
    N --> O
    O -- Yes --> P["Add daft-name filter to all handlers"]
    O -- No --> Q{"exclude_prefix?"}
    P --> Q
    Q -- Yes --> R["Add prefix-exclusion filter to all handlers"]
    Q -- No --> S["refresh_logger()"]
    R --> S

Loading

_{Last reviewed commit: fe9337c}

[cckellogg]

@greptileai

[cckellogg]

@greptileai

[cckellogg]

@greptileai

[codecov]

Codecov Report

❌ Patch coverage is 37.50000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.92%. Comparing base (6bec12d) to head (fe9337c).

Files with missing lines	Patch %	Lines
daft/logging.py	37.50%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6410      +/-   ##
==========================================
+ Coverage   74.78%   76.92%   +2.13%     
==========================================
  Files        1020     1020              
  Lines      136319   136325       +6     
==========================================
+ Hits       101949   104867    +2918     
+ Misses      34370    31458    -2912     

Files with missing lines	Coverage Δ
daft/logging.py	77.77% <37.50%> (-22.23%)	⬇️

... and 62 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[universalmind303]

I don't think this is the right approach. This adds convenience for writing debug logs to a file, but users can already do this with Python's standard logging with just 4 loc:

handler = logging.FileHandler("/path/to/log")
handler.setFormatter(logging.Formatter("..."))
logging.getLogger().addHandler(handler)

setup_logger("debug")

More importantly, the direction we want to move for observability is toward a structured event timeline. giving users visibility into what Daft is actually doing (query planning, partition execution, data reads, etc.) rather than capturing unstructured debug log lines. Adding log_path here commits us to API surface we'd likely deprecate once a proper observability layer is in place.