Skip to content

feat: MaxLinks for pulls #374

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

Merged
merged 3 commits into from
Apr 22, 2023
Merged

feat: MaxLinks for pulls #374

merged 3 commits into from
Apr 22, 2023

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Apr 4, 2023

Uses ipfs/go-graphsync#420

Further comments in Lassie

Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

My biggest concern is a transport option that only works one direction, with no indicator to that effect. I think the right thing is to update graphsync PR to go both directions, and then incorporate that here.

@rvagg rvagg force-pushed the rvagg/maxlinks branch 2 times, most recently from 5a31334 to 99ba9d3 Compare April 6, 2023 12:25
@rvagg
Copy link
Member Author

rvagg commented Apr 6, 2023

Failures discussed in #373, they come from the diff between go-graphsync v0.14.0 and the current main there.

@rvagg rvagg marked this pull request as ready for review April 6, 2023 12:38
rvagg added a commit to filecoin-project/lassie that referenced this pull request Apr 6, 2023
@gammazero gammazero requested a review from hannahhoward April 13, 2023 18:20
@codecov-commenter
Copy link

Codecov Report

Merging #374 (c72405a) into master (131ecef) will increase coverage by 0.02%.
The diff coverage is 70.83%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #374      +/-   ##
==========================================
+ Coverage   61.28%   61.31%   +0.02%     
==========================================
  Files          30       30              
  Lines        3180     3200      +20     
==========================================
+ Hits         1949     1962      +13     
- Misses       1002     1009       +7     
  Partials      229      229              
Impacted Files Coverage Δ
transport/graphsync/graphsync.go 76.02% <70.83%> (-0.38%) ⬇️

Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

LGTM

@hannahhoward hannahhoward merged commit 2dad358 into master Apr 22, 2023
hannahhoward added a commit to filecoin-project/lassie that referenced this pull request Apr 22, 2023
* feat: use traversal link budget instead of limitstore

Ref: ipfs/go-graphsync#420
Ref: filecoin-project/go-data-transfer#374

* chore(deps): update go-data-transfer v2.0.0-rc7

* style(lint): fix fmt

---------

Co-authored-by: hannahhoward <[email protected]>
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.

4 participants