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.

Sign up for GitHub

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

Jump to bottom

Do not send first blocks extension #230

Merged
merged 5 commits into from Sep 29, 2021
Merged

Do not send first blocks extension #230

merged 5 commits into from Sep 29, 2021

Conversation

hannahhoward
Copy link
Collaborator

@hannahhoward hannahhoward commented Sep 29, 2021 •
edited

Goals

Implement #225

Implementation

  • add the extension name an a utility to encode decode the data (just a number)
  • add processing of the extension to the query preparer
  • handle behavior for the extension in the response assembler. You can now specify a number of blocks (** non-unique **) on a request to traverse before they start going over the wire.
  • add integration test to demonstrate flow

@hannahhoward
feat(responseassembler): add response skipping
b7be601
@hannahhoward
feat(responsemanager): process do not send first blocks
a4f1ac3 
define extension and add processing to response manager
@hannahhoward
test(impl): add integration test
03f28ec 
add test demonstrating do not send first blocks flow
@hannahhoward hannahhoward force-pushed the feat/do-not-send-first-blocks-ext branch from 720a3cb to 03f28ec Compare September 29, 2021 22:58
willscott
willscott approved these changes Sep 29, 2021
View reviewed changes
donotsendfirstblocks/donotsendfirstblocks.go Outdated Show resolved Hide resolved
impl/graphsync_test.go Outdated Show resolved Hide resolved
hannahhoward and others added 2 commits September 29, 2021 16:54
@hannahhoward @willscott
Update donotsendfirstblocks/donotsendfirstblocks.go
3c695b7 
Co-authored-by: Will <will.scott@protocol.ai>
@hannahhoward @willscott
Update impl/graphsync_test.go
0127c2b 
Co-authored-by: Will <will.scott@protocol.ai>
@hannahhoward hannahhoward merged commit 57df640 into main Sep 29, 2021
@rvagg
Copy link
Member

rvagg commented Sep 30, 2021

nice, is it go-datatransfer that becomes responsible for using this?

This was referenced Oct 4, 2021
Merged
Merged
Merged
Merged
@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
Closed
80 tasks
@mvdan mvdan deleted the feat/do-not-send-first-blocks-ext branch December 15, 2021 14:18
marten-seemann pushed a commit that referenced this pull request Mar 2, 2023
@raulk
fix log statement. (#230)
c962a5c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willscott willscott willscott approved these changes

@rvagg rvagg Awaiting requested review from rvagg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hannahhoward @rvagg @willscott