Skip to content

Feat: Unify remote filesystem dispatch for exporters (groundwork for lakehouse support) - #1021

Merged
cmgzn merged 4 commits into
mainfrom
pr-a-unified-fs
Aug 12, 2026
Merged

Feat: Unify remote filesystem dispatch for exporters (groundwork for lakehouse support)#1021
cmgzn merged 4 commits into
mainfrom
pr-a-unified-fs

Conversation

@cmgzn

@cmgzn cmgzn commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Motivation

This is the first groundwork PR in the incremental split of #911 (lakehouse data source support). It converges the duplicated "detect path prefix → extract credentials/config → build a PyArrow FileSystem" logic scattered across exporters into a single dispatch extension point. Follow-up Iceberg/Delta/Hudi PRs only need to extend the scheme branches in one place instead of re-implementing per-callsite logic.

Builds on the HDFS support introduced in #1014.

Changes

  • New data_juicer/utils/fs_utils.py with create_filesystem_for_path(path, extra_args=None) -> (fs, remaining_args):
    • dispatches by URI scheme: s3://create_pyarrow_s3_filesystem, hdfs://create_pyarrow_hdfs_filesystem (config includes path so host/port can be inferred), anything else → (None, copy);
    • side-effect-free: never mutates the input extra_args dict; the backend-specific keys (S3_FS_KEYS / HDFS_FS_KEYS) are consumed and removed from the returned remaining_args copy;
    • pyarrow-dependent imports stay lazy (function-level), keeping the module lightweight to import;
    • module docstring documents the convention: future backends (Iceberg/Delta/Hudi) should extend the scheme branches here rather than adding per-callsite prefix logic.
  • Refactor RayExporter: the two parallel S3/HDFS if blocks that popped aws_* / hdfs_* keys from export_extra_args are replaced by a single call to the helper; self.s3_filesystem / self.hdfs_filesystem attributes and their usage in write methods are unchanged.
  • Refactor Exporter: the HDFS branch (PyArrow fs + ArrowFSWrapper) now delegates to the helper; the S3 branch keeps building fsspec storage_options (different backend contract) but its entry check is aligned to the same scheme semantics.
  • Case-insensitive scheme handling: validate_s3_path / validate_hdfs_path and the exporter branch checks now treat the scheme case-insensitively, consistent with urlparse semantics.

Behavior notes

  • Single-slash typos such as s3:/bucket/x or hdfs:/user/x now raise a clear ValueError at exporter construction time (previously they were silently treated as local paths).
  • Uppercase scheme variants (S3://..., HDFS://...) are now correctly accepted and routed to the corresponding filesystem.

Related

Tests

  • New tests/utils/test_fs_utils.py (15 cases): local/relative/file:// paths, S3/HDFS dispatch with mocked factories (exact conf/remaining-key assertions), input-dict immutability (deep-compare), uppercase schemes, query-parameter paths, malformed paths (ValueError), extra_args=None.
  • Extended tests/utils/test_s3_utils.py / test_hdfs_utils.py with uppercase-accept and single-slash-reject cases.
  • Locally green: tests/utils/test_fs_utils.py + test_s3_utils.py + test_hdfs_utils.py (49 passed), tests/core/test_exporter.py + tests/core/test_ray_exporter.py (32 passed), load-strategy HDFS/S3 tests unaffected.

- add create_filesystem_for_path in data_juicer/utils/fs_utils.py as the
  single dispatch point for building PyArrow filesystems by path scheme
  (side-effect-free: never mutates the input extra-args dict and returns
  the remaining args; pyarrow imported lazily)
- refactor S3/HDFS filesystem initialization in Exporter/RayExporter to
  delegate to the unified helper
- make validate_s3_path/validate_hdfs_path and exporter branch checks
  case-insensitive on the scheme; single-slash typos like s3:/x and
  hdfs:/x now raise ValueError instead of being treated as local paths
- add tests/utils/test_fs_utils.py and extend s3/hdfs utils tests
Comment thread data_juicer/core/exporter.py Outdated
Comment thread data_juicer/core/ray_exporter.py Outdated
Comment thread data_juicer/core/exporter.py Outdated
cmgzn added 2 commits August 3, 2026 18:13
Align the remaining scheme checks in the export chain with the
case-insensitive dispatch introduced for exporters, so uppercase
schemes (S3://, HDFS://) are handled consistently end to end:

- exporter/ray_exporter: encryption-skip check and shard-path
  construction now use urlparse(...).scheme.lower(); scheme stripping
  is case-insensitive (split on '://')
- default/ray/partitioned executors: S3 credential injection guard
  uses urlparse(...).scheme.lower()
- config: remote-path detection and log-filename path extraction use
  urlparse (netloc/path) instead of case-sensitive startswith/replace
- file_utils.is_remote_path: case-insensitive scheme check

@fengrui-z fengrui-z left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@cmgzn
cmgzn merged commit 0a7d12c into main Aug 12, 2026
5 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.

3 participants