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

Extract HTTP::Connection ala Reel (WIP: DO NOT MERGE) #72

Closed
wants to merge 1 commit into from

Conversation

tarcieri
Copy link
Member

This is a work-in-progress commit to structure the connection management and pipelining closer to what is presently used today in Reel.

Much of the code is modified copypasta from Reel. I wish there were a more DRY abstraction for this sort of thing (reusing client/server parts of HTTP libraries) but if it exists it presently escapes me.

As there seems to be a lot of interest in this library as of late, I'm posing my (still nowhere near complete) WIP work for some early review. I wish there were a better way to break this change apart into a series of smaller, more easily reviewable changes, but as this is full blown "catching up to Reel" I'm not sure how that's possible.

Comments strongly encouraged!

raise StateError, "can't read in the '#{@request_fsm.state}' request state"
end

@parser.readpartial(size)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is where the EOFError stuff may occur, see #46

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this @parser (HTTP::Response::Parser) is fairly different from the original. Also, this doesn't initiate I/O directly, rather the parser itself does.

This is a work-in-progress commit to structure the connection management and
pipelining closer to what is presently used today in Reel.

Much of the code is modified copypasta from Reel. I wish there were a more DRY
abstraction for this sort of thing (reusing client/server parts of HTTP
libraries) but if it exists it presently escapes me.
@ixti ixti added the WIP label Feb 18, 2014
@sferik sferik mentioned this pull request Mar 4, 2014
@Asmod4n
Copy link
Contributor

Asmod4n commented Mar 4, 2014

Would be nice if you could get the underlying socket as a real IO object, aka .to_io so you could IO.copy_stream it. Some JSON parsers also check if they have a IO object and .read or something similar on it, but only when it is a IO object(aka is_a?(IO), not one which exposes similar methods.

@tarcieri
Copy link
Member Author

tarcieri commented Mar 4, 2014

@Asmod4n we could support a "hijack" API similar to Rack (and Reel) which lets the program take control of the socket. I wouldn't want to directly expose the socket without such an API, as it could end up unsafely shared between user code and the HTTP gem.

@tarcieri tarcieri mentioned this pull request Mar 13, 2014
This was referenced Mar 27, 2014
@ixti ixti mentioned this pull request Apr 5, 2014
@sferik sferik added this to the v0.7 milestone Apr 6, 2014
@ixti ixti modified the milestones: v0.7, v1.0.0 Jan 2, 2015
@sferik
Copy link
Contributor

sferik commented Feb 27, 2015

What more needs to be done before this can be merged?

@tarcieri
Copy link
Member Author

This is pretty out-of-date now (the PR is over a year old!). Honestly I (or someone else) should just start over at this point...

@ixti
Copy link
Member

ixti commented Mar 26, 2015

Closed by #184

@ixti ixti closed this Mar 27, 2015
@sferik sferik modified the milestones: v0.8.0, v1.0.0 Mar 27, 2015
@ixti ixti deleted the connection-class branch March 30, 2015 01:26
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.

5 participants