feat: add support for directory scan mode and enhance file path handling - #674
feat: add support for directory scan mode and enhance file path handling#674Edwardvaneechoud wants to merge 6 commits into
Conversation
✅ Deploy Preview for flowfile-wasm ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Claude finished @Edwardvaneechoud's task in 6m 32s —— View job Claude's review of #674Todo list
SummaryThis is a well-scoped, thoroughly-tested feature. The design consistently gates directory-scan support at three independent layers ( Cross-package contract compliance:
Correctness highlights:
Two non-blocking scalability notes (left inline):
Both look like accepted "slice 1" tradeoffs (the test suite explicitly frames this as slice 1 of the feature, e.g. csv dtype-widening prediction is a documented known gap), so these are flagged as forward-looking considerations rather than blockers. Tests: Coverage is extensive and good — 473+292+241+109+228 new lines across engine behavior, codegen round-trip, executor/state change-detection, cross-package drift, and Not reviewed / limitations of this pass:
No correctness, security, or convention-breaking issues found. |
| # on disk changes, so a cache hit here would serve data the user just replaced. | ||
| if self._source_file_changed(state): | ||
| strategy = self._determine_strategy(run_location) | ||
| return ExecutionDecision(True, strategy, InvalidationReason.SOURCE_FILE_CHANGED) |
There was a problem hiding this comment.
Scalability note (non-blocking): _source_file_changed runs on every _decide_execution call for a read node, and for directory-mode sources it delegates to SourceFileInfo.has_changed() → _aggregate_stats() (flow_node/state.py), which does a fresh recursive glob.glob(..., recursive=True) plus an os.stat() per matched file. This happens synchronously in core, before the cache_results/results_exists short-circuit, on every run — even a run that would otherwise be a pure cache hit.
For a directory with a very large number of files (a common case for "read every file in this folder" workflows — think 100k+ small files), this could add non-trivial latency to every single flow run and briefly block the core process, which conflicts with the "core stays lean / no heavy synchronous work" spirit in CLAUDE.md (even though this isn't a .collect()).
Given this is called out as "slice 1" in the test docstrings, this is likely an accepted tradeoff, but it might be worth a follow-up (e.g., caching the aggregate stat scan for a short TTL, or capping the walk) before this sees very large directories in the wild.
| encoding="utf8-lossy", | ||
| ignore_errors=True, | ||
| **fallback_infer, | ||
| **extra, |
There was a problem hiding this comment.
Minor note: _assert_uniform_columns opens every matched file (sequentially) just to read its schema via collect_schema() before the real scan happens. For a directory scan over many parquet/ipc files this is an O(n) metadata-read pass in addition to the actual scan. It's a reasonable correctness-first tradeoff (documented well in the docstring), but for directories with thousands of files it could meaningfully slow down every run of the node. Worth keeping in mind if a later slice adds pagination/partitioned datasets with large file counts.
|
Claude finished @Edwardvaneechoud's task in 1m 29s —— View job Claude's review of #674Todo list
SummaryI re-checked this PR after the Given that, my prior review of this PR stands:
Both are framed in the tests as accepted "slice 1" tradeoffs, not blockers. Not re-verified in this pass: I still could not execute No new action needed from me unless new commits are pushed or you'd like me to act on the scalability notes above. |
|
Claude finished @Edwardvaneechoud's task in 3m 10s —— View job Claude's review of #674Todo list
SummaryI re-reviewed the current HEAD ( Design holds up well on a second pass:
Same two non-blocking scalability notes as the previous pass on this PR still apply (unchanged code, already flagged inline):
Both remain reasonable "slice 1" tradeoffs given the test suite explicitly frames this as slice 1 (e.g. the documented gap that csv dtype-widening isn't predicted across files in the schema callback). Not reviewed / limitations of this pass:
No new correctness, security, or convention issues found on this pass. |
|
Claude finished @Edwardvaneechoud's task in 50s —— View job Claude's review of #674Todo list
SummarySince the last review pass (at
No new correctness, security, or convention issues — this commit is a pure documentation/comment tidy-up with zero behavioral impact, so all prior findings on this PR still stand:
Not re-verified in this pass: couldn't execute |
|
Claude finished @Edwardvaneechoud's task in 1m 23s —— View job Claude's review of #674Todo list
SummaryCommit What changed and why it's correct:
No new issues found. All the changes in this commit are narrowly scoped to the previously-flagged concerns, come with dedicated new tests for each behavior change, and don't introduce new cross-package contract violations (worker still never executes a directory scan; core still never Not re-verified in this pass: still could not execute |
This pull request introduces comprehensive support for directory-mode file reading and schema inference throughout the codebase, improving robustness and flexibility when handling multiple files as a single logical dataset. The changes include new utility functions for expanding glob patterns, validation checks for directory scans, and refactoring of code generation and data engine logic to support these features. Additionally, the code now better handles edge cases, such as zero matched files and inclusion of source-path columns.
The most important changes are:
Directory Scan Support and Validation
assert_directory_scan_supported) in code generation, data engine, and flow graph logic to ensure that directory scans are only allowed for supported file types and encodings, preventing runtime errors. [1] [2] [3] [4] [5]Glob Pattern Expansion and Source Resolution
_resolve_scan_sourceand related helpers to expand glob patterns for directory-mode reads, ensuring that file lists are resolved before passing to polars, and raising a specific error if no files match.Code Generation Enhancements
Schema Inference for Directory Mode
Robustness and Consistency Improvements
These changes collectively make directory-based data ingestion safer, more predictable, and easier to maintain across the codebase.