Skip to content

Use fsspec for remote cache downloads - #758

Merged
d-chambers merged 1 commit into
devfrom
agent/fsspec-only-remote-cache
Jul 19, 2026
Merged

Use fsspec for remote cache downloads#758
d-chambers merged 1 commit into
devfrom
agent/fsspec-only-remote-cache

Conversation

@d-chambers

@d-chambers d-chambers commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Description

Use fsspec/universal-pathlib as the single transport for remote cache materialization.

The HTTP cache fallback previously opened a second urllib connection because materialization could begin while the failed fsspec handle was still open. This duplicated HTTP behavior and required manually translating only a subset of fsspec storage options.

This change closes the failed remote handle before materialization, then streams HTTP resources through UPath.open(..., block_size=0). It removes the urllib-specific header/auth translation and the blocking-download timeout setting that only applied to that alternate stack. The existing atomic cache write and configured download chunk size remain unchanged.

Regression tests verify that HTTP materialization uses the UPath/fsspec transport and that the failed remote handle is closed before the local cache opener runs.

Validation:

  • pytest tests -m "not network" -q — 7,735 passed
  • pytest tests -m network -q — 100 passed
  • pytest dascore --doctest-modules -q — 142 passed
  • pre-commit run --all-files — passed

Changelog

none

Checklist

I have (if applicable):

  • referenced the GitHub issue this PR closes.
  • documented the new feature with docstrings and/or appropriate doc page.
  • included tests. See testing guidelines.
  • added the "ready_for_review" tag once the PR is ready to be reviewed.

Summary by CodeRabbit

  • Bug Fixes
    • Improved remote file downloading across supported HTTP and S3-compatible storage backends.
    • Enhanced fallback handling so remote connections close cleanly when switching to cached local files.
    • Removed the remote download timeout option from runtime configuration; downloads now use the underlying storage connection’s behavior.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Remote download transport

Layer / File(s) Summary
Resource-based remote download
dascore/config.py, dascore/utils/remote_io.py, tests/test_utils/test_io_utils.py
Removes the remote timeout setting and urllib fallback, uses resource.open(...) for all protocols, and updates tests for resource-based streaming and block sizes.
Fallback handle switching
dascore/utils/remote_io.py, tests/test_utils/test_io_utils.py
Closes the remote handle before reopening the cached local handle and verifies this behavior while preserving position.

Possibly related PRs

  • DASDAE/dascore#741: Directly overlaps with the remote HTTP download fallback and its header/authentication test coverage.

Suggested labels: IO

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and accurately summarizes the main change to remote cache downloads.
Description check ✅ Passed The PR description matches the template and includes the required summary, checklist, and validation details.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/fsspec-only-remote-cache

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.

@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (2aa3c00) to head (2ee02a6).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff            @@
##               dev      #758   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          161       161           
  Lines        16619     16587   -32     
=========================================
- Hits         16619     16587   -32     
Flag Coverage Δ
network 49.65% <100.00%> (-0.01%) ⬇️
unittests 99.98% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@d-chambers
d-chambers marked this pull request as ready for review July 19, 2026 17:56
@d-chambers d-chambers added the ready_for_review PR is ready for review label Jul 19, 2026 — with ChatGPT Codex Connector
@coderabbitai coderabbitai Bot added the IO Work for reading/writing different formats label Jul 19, 2026
@d-chambers
d-chambers merged commit 0313085 into dev Jul 19, 2026
29 checks passed
@d-chambers
d-chambers deleted the agent/fsspec-only-remote-cache branch July 19, 2026 18:13
@d-chambers d-chambers removed the ready_for_review PR is ready for review label Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

IO Work for reading/writing different formats

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant