-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Set correct content-type for ipfs get #2673
Conversation
test_expect_success "ipfs get response has the correct content-type" ' | ||
curl -I "http://localhost:$PORT_API/api/v0/get?arg=$HASH" | grep "^Content-Type: application/x-tar" | ||
' | ||
|
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.
Should have a negative test in here, too.
14bb435
to
ddb8585
Compare
Any idea why this test fails all over the board: https://travis-ci.org/ipfs/go-ipfs/jobs/134285028#L6773-L6779 ? |
I have no idea. @whyrusleeping? |
the tests that fail are ipfs config? Thats weird... what content type is being returned through that command? |
When running on this branch:
|
@RichardLitt the reason this breaks the config command is because So here lies the issue. Should the output type of |
This isn't for |
@RichardLitt re-read my comment. You made a change that touches every command, that had some side effects. Take a look at the assumptions the old code made, and compare it to what your change does. |
I would say no. It should be JSON. But I am not sure. |
@RichardLitt Yeah, so thats the problem. The json formatter produces minified output, If a commands normal content type is JSON, then this PR will cause that command to produce the condensed minified output. |
Could we add an option to pretty-print, as desired? |
What is follow up for this? |
I am not attached to it. #2004 was not originally my pull request, and was closed without comment; I was trying to save what good there was in it. I don't feel familiar enough with the specific use case or the go-ipfs codebase right now to adequately change this. Feel free to close. |
This is a version of #2004, with CR from @jbenet integrated in, and should close #1824
Related to ipfs-inactive/js-ipfs-http-client#263.
License: MIT
Signed-off-by: Richard Littauer richard.littauer@gmail.com