More fine grained response controls#71
Merged
hannahhoward merged 5 commits intomasterfrom Jul 2, 2020
Merged
Conversation
ribasushi
approved these changes
Jun 29, 2020
ribasushi
left a comment
There was a problem hiding this comment.
Look good overall, two low-medium nits. Ok to merge as-is.
Comment on lines
+275
to
+279
| // PauseResponse pauses an in progress response (may take 1 or more blocks to process) | ||
| PauseResponse(peer.ID, RequestID) error | ||
|
|
||
| // CancelResponse cancels an in progress response | ||
| CancelResponse(peer.ID, RequestID) error |
There was a problem hiding this comment.
Since this is an interface, would it perhaps make sense to proactively add ...ExtensionData here too? To reduce churn don the road...
Collaborator
Author
There was a problem hiding this comment.
possibly? Gonna not do it now but may revisit
|
|
||
| func (prm *processRequestMessage) handle(rm *ResponseManager) { | ||
| for _, request := range prm.requests { | ||
| key := responseKey{p: prm.p, requestID: request.ID()} |
There was a problem hiding this comment.
General observation, ignore if no time to fix wholesale:
The contraction of peerID to p not only during varnaming but also in the struct-member names is fundamentally confusing. Especially given that the rest of the struct members have full-length names.
Collaborator
Author
There was a problem hiding this comment.
that's a reasonable thought. I will address seperately.
Add the ability for anyone who knows a requestID & peer to pause a response at any time
add function to directly cancel responses from requestID. also, move query execution code to seperate struct, internal for now
Support sending extensions when resuming a request
0dd0e78 to
7893d4a
Compare
marten-seemann
pushed a commit
that referenced
this pull request
Mar 2, 2023
* Emit events with received cids (#71) * persist received cids on channel state. * Send, Receive and Validate Restart requests (#75) * Send, Receive and Validate Requests * Initiating and Responding Tests and bug fixes (#76) * Testing for resuming data transfer work * Cleanup Push Restarts PR (#79) * cleanup of restart PR * link the peers * Tests for pull restarts (#84) * tests for pull restarts * Merge Tests cleanup work (#92) * cleanup of restart PR * cleanup timedout channels (#93) * backward compatibility of restart (#96) * backward compatibility of restart * changes and tests * more tests * better error handling for restarts * feat(message): switch to cbor map encoding (#97) switch to cbor map encoding for the 1_1 message protocol * feat(channels): setup datastore migrations (#99) setup datatransfer channels so they migrate over successfully Co-authored-by: Hannah Howard <hannah@hannahhoward.net>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Goals
Allow users of module (particularly those listening through hooks) to have more fine grained control of responding to requests
Implementation