Skip to content

fix(filesystem): list files through every registered scheme - #274

Open
hampsterx wants to merge 1 commit into
panodata:mainfrom
hampsterx:fix/filesystem-scheme-listing
Open

fix(filesystem): list files through every registered scheme#274
hampsterx wants to merge 1 commit into
panodata:mainfrom
hampsterx:fix/filesystem-scheme-listing

Conversation

@hampsterx

Copy link
Copy Markdown
Contributor

Summary

  • Read from r2://, oss://, hdfs://, smb://, ftp://, dbfs://, oci:// and webhdfs://. Listing raised KeyError: '<scheme>' on the first matched file, so those sources could construct but never load anything.
  • Resolve a file's modification date from the listing the filesystem client returns, rather than from a table keyed by URL scheme that only carries the schemes dlt itself ships.
  • Report a listing with no usable date by naming the scheme, the keys present and why each candidate was rejected, instead of raising KeyError on one of them.

Changes

  • Add dlt_filesystem/source/lister.py, holding glob_files (vendored from dlt.common.storages.fsspec_filesystem) plus resolve_modification_date. dlt's own per-scheme extractor is consulted first, so every scheme it knows keeps its exact behaviour.
  • Pair each fallback key with the decoder its backend uses, taken from that backend's own modified(): WebHDFS counts milliseconds, FTP reports an RFC 3659 modify fact, fsspec-databricks names the key modified, ossfs and s3fs use LastModified, ocifs uses timeModified.
  • Resolve mtime last, since SMB carries both it and time, where time is the access time.
  • Reject the year-less Aug 4 09:30 that fsspec parses out of dir output on FTP servers without MLSD, because it cannot name an instant.
  • Leave out dlt's HuggingFace cache-invalidation branch, since no hf:// scheme is registered.

This also covers a client whose listing uses a different key than its scheme is mapped to, which is what any pyarrow.fs client does (ArrowFSWrapper reports mtime whatever the backend). That is the discovery-side blocker for GH-233.

Test plan

  • Full Docker-free suite: 1004 passed, 44 skipped
  • Remote emulator suite: 25 passed in tests/main/filesystem/test_remote_integration.py, including a new S3-compatible read through r2://
  • Regression proof: pointing the adapter back at dlt's glob_files fails the new r2 test with KeyError: 'r2'
  • Resolver covered per scheme with each backend's real value encoding (epoch millis, MLSD string, epoch seconds, datetime)
  • Ruff formatting/checks, ty, metadata validation
  • Strict Sphinx build
  • Independent Codex and GLM-5.2 reviews; both found the same three encoding defects, all confirmed against the backends' sources and fixed

References

Reading from `r2://`, `oss://`, `hdfs://`, `smb://`, `ftp://`, `dbfs://`,
`oci://` or `webhdfs://` raised `KeyError: '<scheme>'` on the first matched
file, so those sources could construct but never load anything.

File discovery resolved each file's modification date through dlt's table of
per-scheme extractors, where an entry reads one backend-specific key out of the
listing (`LastModified` for `s3`, `updated` for `gs`, `last_modified` for `az`).
A scheme absent from that table has no entry to read, and only the schemes dlt
itself ships are present.

Resolve the date from the listing instead: consult dlt's extractor first, so
every scheme it knows keeps its exact behaviour, then read the key the backend
emits and decode it the way that backend encodes it. Each pairing is taken from
the backend's own `modified()`, which reads the same key: WebHDFS counts in
milliseconds, FTP reports an RFC 3659 `modify` fact, Databricks names it
`modified`. Prefer a specific key over `mtime`, because SMB carries both that
and `time`, and `time` is the access time.

This also covers a client whose listing uses a different key than its scheme is
mapped to, which is what any `pyarrow.fs` client does.

A listing with no usable date names the scheme, the keys present and why each
candidate was rejected, rather than raising `KeyError` on one of them. FTP
servers without MLSD land there: fsspec parses their `dir` output into a
year-less `Aug 4 09:30`, which cannot name an instant.

Cover the resolver per scheme with each backend's own value encoding, and read
an S3-compatible bucket through `r2://` end to end against the emulator.
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • coderabbit-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e7f657b6-6c39-4573-804d-1032d538d9cd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@read-the-docs-community

Copy link
Copy Markdown

Documentation build overview

📚 omniload | 🛠️ Build #33899586 | 📁 Comparing 096edc6 against latest (a9b8429)

  🔍 Preview build  

1 file changed
± changelog.html

@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.56716% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.38%. Comparing base (a9b8429) to head (096edc6).

Files with missing lines Patch % Lines
src/dlt_filesystem/source/lister.py 86.15% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #274      +/-   ##
==========================================
+ Coverage   60.23%   60.38%   +0.15%     
==========================================
  Files         236      237       +1     
  Lines       11157    11223      +66     
==========================================
+ Hits         6720     6777      +57     
- Misses       4437     4446       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@amotl amotl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your discoveries and improvements in this area.
NB: 60% code coverage milestone reached. 💯

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