This repository has been archived by the owner on Jun 27, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 53
decouple the DAG traversal logic from the DAG reader (local branch) #60
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
assigned magik6k
Jan 28, 2019
magik6k
changed the title
Feat/dag reader/use walker
decouple the DAG traversal logic from the DAG reader (local branch)
Jan 28, 2019
magik6k
force-pushed
the
feat/dag-reader/use-walker
branch
from
January 28, 2019 17:30
343064e
to
93c3a88
Compare
schomatis
reviewed
Jan 28, 2019
@@ -472,7 +472,7 @@ func (dr *dagReader) Seek(offset int64, whence int) (int64, error) { | |||
// searches in the `Walker` (see `Walker.Seek`). | |||
|
|||
case io.SeekEnd: | |||
return dr.Seek(int64(dr.Size())-offset, io.SeekStart) | |||
return dr.Seek(int64(dr.Size())+offset, io.SeekStart) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
schomatis
reviewed
Jan 28, 2019
schomatis
reviewed
Jan 28, 2019
schomatis
approved these changes
Jan 28, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@magik6k Thanks a lot (again!) for carrying this forward ❤️
4 tasks
magik6k
force-pushed
the
feat/dag-reader/use-walker
branch
from
January 29, 2019 16:53
93c3a88
to
52e19dc
Compare
Decouple the DAG traversal logic from the `PBDagReader` encapsulating it in the (new) `Walker` structure. Collapse PB and Buffer DAG readers into one (`dagReader`) removing the `bufdagreader.go` and `pbdagreader.go` files, moving all the code to `dagreader.go`. Remove `TestSeekAndReadLarge` and `TestReadAndCancel` which operated directly on the `NodePromise` structure that is now abstracted away in `NavigableIPLDNode`, in the `go-ipld-format` repo, where they should be recreated. License: MIT Signed-off-by: Lucas Molas <schomatis@gmail.com>
This version writes one node at a time instead of first buffering the entire file contents in a single array (taking up too much memory) before actually writing it.
magik6k
force-pushed
the
feat/dag-reader/use-walker
branch
from
January 29, 2019 23:28
52e19dc
to
cdb64d5
Compare
I ran go-ipfs gotest+sharness with this applied and it passed. @Stebalien mind giving this one last look before merging? |
Stebalien
approved these changes
Jan 30, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, this is correctly replicating the current behavior.
🚀 🎉 |
Thank you both @magik6k and @Stebalien! |
This was referenced Jan 30, 2019
Jorropo
pushed a commit
to Jorropo/go-libipfs
that referenced
this pull request
Jan 25, 2023
…lker decouple the DAG traversal logic from the DAG reader (local branch) This commit was moved from ipfs/go-unixfs@f0d9ddc
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes some issues with #12 (I couldn't push there), also adds a fix for SeekEnd from ipfs/kubo#5934