-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
http api: makes sure header is sent even when r is not ready yet. fixes #3304 #3305
Conversation
License: MIT Signed-off-by: Jan Winkelmann <j-winkelmann@tuhh.de>
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.
Great catch, this should fix quite a few 'apparent' hanging issues. Just one change then LGTM
@@ -298,6 +298,9 @@ func flushCopy(w io.Writer, r io.Reader) error { | |||
return err | |||
} | |||
for { | |||
// flush to send header when r is not ready yet | |||
f.Flush() |
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.
probably want to check and handle the error here, just in case
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.
Flush doens't return an error, that's why the tests failed :D
License: MIT Signed-off-by: Jan Winkelmann <j-winkelmann@tuhh.de>
I now check the error. Also I removed the flush at the bottom of the loop so it doesn't flush pointlessly. |
Tests are failing now... i'm guessing removing that final flush wasnt quite right |
License: MIT Signed-off-by: Jan Winkelmann <j-winkelmann@tuhh.de>
License: MIT Signed-off-by: Jan Winkelmann <j-winkelmann@tuhh.de>
Can you squash it, we don't want to have any commits with build/test failures as it makes git bisecting much harder. |
@Kubuxu uh, sorry. next time! |
* commit '6f3ae5da293f971c09e3e6fc0763aeca04ba98d3': rename DataSize to FileSize update HashOnRead validation to properly support cids http api: makes sure header is sent even when r is not ready yet. fixes ipfs#3304 (ipfs#3305) fix add/cat of small files raw dag: make raw nodes work in cat and get, add tests feat: make metrics injection log an error instead of warning test: check if metrics work in sharness Fix metrics being injected after node initalization unixfs: allow use of raw merkledag nodes for unixfs files ds-help: add helper func to convert from Cid to DsKey and the reverse cli: refactor to expose argument parsing functionality # Conflicts: # core/commands/add.go
Otherwise API clients block on pubsub subscriptions until the first packet is sent (see, well, #3304).