Small HTTPServer changes #279

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

eklitzke commented Jun 12, 2011

These are some small changes to IOStream, HTTPServer, and HTTPConnection that I made to facilitate extra callbacks I need for another project I'm using tornado for (for context, I PM'ed bdarnell to ask for some guidance with this earlier in the week).

I'm kind of shooting in the dark here with respect to code style and whatnot. I split the change up into three separate commits that should all be independent, and are useful to me in different ways. If any of the commits don't look OK, you can just pull in the other ones. I'm also open to suggestions about alternate ways to implement the changes.

Thanks!

Owner

bdarnell commented Jul 3, 2011

OK, I'm coming back to this change now.

For multiple close callbacks, there needs to be a way to remove a callback once it's set. It's currently done with set_close_callback(None) (e.g. web.py:623). That's a public interface, but it's obscure enough that I wouldn't feel terrible about breaking it. Like IOLoop.{add,remove}_timeout, the set/add method needs to return a handle that is passed to the remove method (because of stack_context wrapping).

on_headers should be renamed and called before the request_callback in _on_request_body as well.

Why does the handle_stream change also split the HTTPConnection constructor into init and start methods?

Overall, it feels a little complicated and fragile to have to subclass HTTPServer and HTTPConnection when only a few methods are really safe to override. I wonder if it would be better to have the server take an object that implements an HTTPServerListener interface (ugh... Java flashbacks :)). Another possibility would be to wrap Application like I had originally suggested, but also add some sort of finish hook to HTTPRequest.

eklitzke closed this Oct 4, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment