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

Fix unixfs fetch #364

Merged
merged 4 commits into from
Mar 9, 2022
Merged

Fix unixfs fetch #364

merged 4 commits into from
Mar 9, 2022

Conversation

hannahhoward
Copy link
Collaborator

@hannahhoward hannahhoward commented Mar 7, 2022

Goals

Fix GraphSyncing UnixFS for multiblock files
based on test in #363

Implementation

  • update go-ipld-prime to master (need to tag)
  • update go-unixfsnode to master, then further apply fix contained in Add unixfs to UnixFS path selector tail go-unixfsnode#28
  • modify the go-type for the ipld message v2 format, making extensions a map[string]*datamodel.Node instead of a map[string]datamodel.Node per @mvdan
  • modify the visitors on both sides of the request execution to read the entire file into ioutil.Discard, which triggers the loading of the files blocks. This causes the responder to send them over the wire and the requestor to verify them.
  • also, WOW -- did I not commit a bunch of changes to the docs? HA. Oh, right, I probably left them unsaved in VSCode.

For discussion

  • blocked on tagged go-ipld-prime
  • blocked on merge of unixfs PR and retagging

tchardin and others added 3 commits March 3, 2022 10:18
support pathing into multiblock unixfs files, properly reading the files inside the visitor so that
they go over the wire
@@ -134,6 +137,16 @@ func (rm *RequestManager) requestTask(requestID graphsync.RequestID) executor.Re
Root: cidlink.Link{Cid: ipr.request.Root()},
Selector: ipr.request.Selector(),
Visitor: func(tp traversal.Progress, node ipld.Node, tr traversal.VisitReason) error {
if lbn, ok := node.(datamodel.LargeBytesNode); ok {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems like the relevant chunk for the partial traversal. looks good

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think so right? in the meantime it fixes multiblock files as well.

@hannahhoward hannahhoward merged commit f4afe52 into main Mar 9, 2022
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.

None yet

3 participants