Skip to content

Accept IO objects as HTTP request bodies#2096

Closed
Acconut wants to merge 4 commits intocrystal-lang:masterfrom
Acconut:feat/http_body_io
Closed

Accept IO objects as HTTP request bodies#2096
Acconut wants to merge 4 commits intocrystal-lang:masterfrom
Acconut:feat/http_body_io

Conversation

@Acconut
Copy link
Copy Markdown

@Acconut Acconut commented Feb 1, 2016

Previously only strings (or nil) would have been accepted by the HTTP::Client.exec methods.

@Acconut
Copy link
Copy Markdown
Author

Acconut commented Feb 3, 2016

Tests pass now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this rather raise if it's an IO, or at least read it?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oh my bad, I thought to_s returned the entire content as a String but this seems to be only the case for MemoryIO and not File etc. I believe the most intuitive thing is that we read it without raising an error.

@asterite
Copy link
Copy Markdown
Member

asterite commented Feb 3, 2016

This is a good change, we forgot about request's body being able to be an IO.

We should also think if we want to have an IO in the server, so I guess this will need some more thought from our side.

@asterite
Copy link
Copy Markdown
Member

This was implemented in #3406

@asterite asterite closed this Oct 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants