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.

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

only set stream header on streamed output #1531

Merged
merged 2 commits into from
Jul 28, 2015
Merged

only set stream header on streamed output #1531

merged 2 commits into from
Jul 28, 2015

Conversation

whyrusleeping
Copy link
Member

fixes #1528

License: MIT
Signed-off-by: Jeromy jeromyj@gmail.com

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
@jbenet jbenet added the status/in-progress In progress label Jul 28, 2015
@whyrusleeping whyrusleeping added this to the IPFS 0.3.6 milestone Jul 28, 2015
@@ -182,6 +174,11 @@ func sendResponse(w http.ResponseWriter, req cmds.Request, res cmds.Response) {
h.Set(contentLengthHeader, strconv.FormatUint(res.Length(), 10))
}

if _, ok := res.Output().(io.Reader); ok {
mime = ""
Copy link
Member

Choose a reason for hiding this comment

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

preserve the comment, else we won't know why the hell we're setting mime = "" in a few weeks. 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

but i didnt like that comment... :(

@GitCop
Copy link

GitCop commented Jul 28, 2015

There were the following issues with your Pull Request

  • Commit: 98559d0
    • Invalid signoff. Commit message must end with License: MIT
      Signed-off-by: .* <.*>

Guidelines are available to help. Your feedback on GitCop is welcome on this issue


This message was auto-generated by https://gitcop.com

@whyrusleeping
Copy link
Member Author

omg gitcop, shut up. If i want to use githubs online editor to write code, I can do so. I am an adult.

@jbenet
Copy link
Member

jbenet commented Jul 28, 2015

@whyrusleeping then put the trailer in manually. don't blame gitcop, gitcop's right.

// we set this header so clients have a way to know this is an output stream
// (not marshalled command output)
mime = ""
h.Set(streamHeader, "1")
Copy link
Member

Choose a reason for hiding this comment

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

so only set the header when it is an io.Reader and mime is off?

is this right?. why doesnt a chan interface{} have "StreamHeader: 1" too? (it is written out like a stream). what does streamHeader mean exactly?

Copy link
Member Author

Choose a reason for hiding this comment

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

streamheader means that youre getting an arbitrary stream of data. chan interface{} means youre getting a series of json objects.

jbenet added a commit that referenced this pull request Jul 28, 2015
only set stream header on streamed output
@jbenet jbenet merged commit 681da0a into master Jul 28, 2015
@jbenet jbenet removed the status/in-progress In progress label Jul 28, 2015
@jbenet jbenet deleted the fix/stream-header branch July 28, 2015 20:04
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.

webui broken by get fix
3 participants