Use a bitswap session for 'Cat' #4641
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 
whyrusleeping
force-pushed
the
feat/cat-sessions
branch
from
704ec60
to
197e60f
Compare
|
If we get this working though, it should make |
Stebalien
reviewed
Feb 2, 2018
There was a problem hiding this comment.
The other approach would be to just make a hybrid DagService implementation that passes all Get calls to an underlying session, and then Add calls get routed normally.
I agree. This also makes progress towards #4138.
I'd kind of like a simple hamt reader (so we traverse read-only filesystems) but we can do that later.
merkledag/merkledag.go
Outdated
| @@ -146,6 +146,15 @@ func (sg *sesGetter) GetMany(ctx context.Context, keys []*cid.Cid) <-chan *ipld. | |||
| return getNodesFromBG(ctx, sg.bs, keys) | |||
| } | |||
|
|
|||
| func NewSession(ctx context.Context, dag ipld.DAGService) ipld.NodeGetter { | |||
There was a problem hiding this comment.
- This should probably take a NodeGetter (unless we make sessions full DAGServices).
- Docs are nice. What happens when the context is canceled? What happens to in-flight requests (esp if we just return the dag and not a session)?
There was a problem hiding this comment.
Alternatively, we could define a new interface:
type Session interface {
Session(ctx context.Context) ipld.NodeGetter
}
func NewSession(ctx context.Context, dag ipld.NodeGetter) ipld.NodeGetter {
if ses, ok := dag.(Session); ok {
return ses.Session(ctx)
}
return dag
}
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
whyrusleeping
force-pushed
the
feat/cat-sessions
branch
from
197e60f
to
1de040d
Compare
Stebalien
approved these changes
Feb 3, 2018
|
(except the missing documentation) |
magik6k
approved these changes
Feb 3, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I'm actually not happy with how this turned out. Everything worked out fine until I had to deal with the Hamt code. Theres no distinction between reading from a hamt and writing to it. The solution i chose here was to separate that out, its a bit messy. The other approach would be to just make a hybrid
DagServiceimplementation that passes all Get calls to an underlying session, and thenAddcalls get routed normally. I think that method seems a lot cleaner...cc @Stebalien @magik6k @kevina let me know what you all think.
License: MIT
Signed-off-by: Jeromy jeromyj@gmail.com