Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for range requests to Fetch #182

Merged
merged 3 commits into from
Jan 26, 2025
Merged

Conversation

james-pre
Copy link
Member

@james-pre james-pre commented Jan 25, 2025

This is a PR to track the progression of #53.

The implementation in 50e600d involves some changes to transactions that are not backwards type compatible and not runtime forward compatible. Due to the significance of these changes, I've labeled the PR as breaking.

Tests have not been changed at all and are passing, so that is good at least.

@james-pre james-pre added enhancement New feature or request breaking This issue or pull request involves a breaking change labels Jan 25, 2025
@james-pre james-pre self-assigned this Jan 25, 2025
@james-pre james-pre linked an issue Jan 25, 2025 that may be closed by this pull request
@james-pre
Copy link
Member Author

One issue that will likely need to be fixed is making sure the cache can keep track of which ranges have been fetched. At the moment, the un-fetched ranges would be zeroed out, and the current behavior would result in a read of the zeroed data without fetching it first.

@zardoy, this will likely tie into streaming zip files, or at least fetching them in parts. @coder0107git mentioned this in zen-fs/archives#3. Do either of you have any feedback?

@mcandeia, I know you aren't using Fetch directly, but this PR also impacts the underlying transaction structure and behavior. Are you depending on this in any way? If so, would you be willing to upgrade even with breaking changes to the internal API? It would mean you can do partial reads and writes and the transaction level. Please let me know your thoughts.

@zardoy
Copy link

zardoy commented Jan 25, 2025

this will likely tie into streaming zip files, or at least fetching them in parts. @coder0107git mentioned this in zen-fs/archives#3. Do either of you have any feedback?

I just want zip data so badly. Yes making fetch automatically work with this would be beneficial

@james-pre
Copy link
Member Author

james-pre commented Jan 25, 2025

I think a nice solution to the caching problem I mentioned earlier is adding the offset/end to fetchFile, which can be used by ZipFS. Then fetchFile can handle the reads, writes, and storage.

@james-pre
Copy link
Member Author

james-pre commented Jan 26, 2025

The actual support for range requests has been implemented over on Utilium (james-pre/utilium@4c82012) so it can be used in other projects (not just ZenFS).

@james-pre james-pre merged commit a0c61c2 into main Jan 26, 2025
1 check passed
@james-pre james-pre deleted the fetch-range-requests branch January 26, 2025 05:40
@zardoy
Copy link

zardoy commented Jan 26, 2025

I just want zip data so badly.

Sorry I meant zip data streaming (zen-fs/archives#3)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This issue or pull request involves a breaking change enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fetch: Support HTTP Range Requests
2 participants