-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[WIP] HTTP::Server refactor #2009
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
Conversation
|
This looks pretty cool! I'll give this try with Kemal middlewares. Meanwhile how do i port something like this to new one https://github.com/sdogruyol/kemal/blob/master/src/kemal/middleware/http_basic_auth.cr ? Is passing around |
|
I like the idea! And code looks 👍 |
|
@sdogruyol It changes to this: https://gist.github.com/asterite/57f9d71af44573d4c0db. Basically:
|
|
Thanks @asterite . That's great! This is not related to issue but how can i profile a code with Instrumentation tool on OS X? Because i'm getting performance degradation after Crystal 0.10.0 and i wanna investigate that. |
|
@sdogruyol I use Instruments, selecting the Time Profiler option. https://developer.apple.com/library/mac/documentation/AnalysisTools/Reference/Instruments_User_Reference/TimeProfilerInstrument/TimeProfilerInstrument.html |
|
Thanks 👍 |
b781529 to
9dc0f4f
Compare
|
Thank you @asterite for working on this!!! ❤️ ❤️ ❤️ The modification suggested here will open the door to functionality like HTTP/2 Server Push. There is a rich discussion about this between Ilya Grigorik, Aaron Patterson and others in relation to Ruby's Rack implementation of HTTP/2 spec: I believe is worth taking a look since redesign of HTTP::Server interface and request/response cycle will need to accommodate HTTP/2 support in the future. |
This PR changes the way
HTTP::ServerandHTTP::Handlerwork. Instead of having each handler return a Response object that provides the body, headers, etc., now each handler receives a context that has both the request and an initial response, to which you can write. This makes it super simple to stream a response. For example:This will print 0, 1, 2... endlessly on the response. Try to do that with the current approach and you'll see it's really hard.
Or, imagine you want to write a JSON object directly to the response (without building it in memory first), or a CSV file. Right now it's really hard, you have to create some sort of IO from which you read the contents.
Another good thing about this change is that
HTTP::Server::Contexthasrequestandresponseproperties, but you can add more properties to it so handlers down the chain have access to these. For example a handler that sets asessionproperty.The downside of this change is that response headers must be set before writing the body. There's also the thing that now we have
HTTP::Client::Response(returned by an HTTP::Client request) andHTTP::Server::Response, to which you can write the body. But the advantages of this change are much more important than this little downside./cc @ysbaddaden @jhass @sdogruyol