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

feat: Refactor unixfs #606

Merged
merged 4 commits into from
Dec 19, 2022
Merged

feat: Refactor unixfs #606

merged 4 commits into from
Dec 19, 2022

Conversation

ppodolsky
Copy link
Contributor

@ppodolsky ppodolsky commented Dec 15, 2022

ResponseClip has been deleted but it is better to discuss. I did it because its semantics was not obvious, it was used for clipping both entire response and partial responses from Node with non-trivial transitions between them.

Worth to note, now Range header with value start-end translates to rust struct Range with values start and end + 1. Range in rust non-inclusive for right bound, it is more convinient.

@ppodolsky ppodolsky force-pushed the feature/unixfs branch 5 times, most recently from d79bda0 to bc7f4cd Compare December 16, 2022 10:01
@ppodolsky ppodolsky changed the title [WIP] feat: Refactor unixfs feat: Refactor unixfs Dec 16, 2022
@ppodolsky ppodolsky force-pushed the feature/unixfs branch 6 times, most recently from c755db2 to 92f96a4 Compare December 19, 2022 21:18
Copy link
Collaborator

@Arqu Arqu left a comment

Choose a reason for hiding this comment

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

Thank you for the deep work here. VERY subtle bugs you caught out.
And magnificent improvements on the seeking.

@@ -103,11 +103,14 @@ pub fn parse_range_header(range: &HeaderValue) -> Option<Range<u64>> {
if start >= end || end == 0 {
return None;
}
Some(Range { start, end })
Some(Range {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ugh, this is a rough bug on my end.

ResponseClip::Clip(bytes_reader.bytes.len()),
&bytes_reader.bytes,
Some(bytes_reader.bytes.len()),
&bytes_reader.bytes[pos_current..],
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@Arqu Arqu merged commit 51e3ddf into n0-computer:main Dec 19, 2022
ppodolsky added a commit to izihawa/iroh that referenced this pull request Dec 20, 2022
* feat: Refactor unixfs

* pr: fixes

* fix: Document fix and correct default for absent gateway path

* feat: Skip blocks on seeking
ppodolsky added a commit to izihawa/iroh that referenced this pull request Dec 28, 2022
* feat: Refactor unixfs

* pr: fixes

* fix: Document fix and correct default for absent gateway path

* feat: Skip blocks on seeking
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