chris-morgan
Implement `send` method to send content along with the request
@FlaPer87 Sorry I haven't followed this and #9 up; I got quite busy for a while, finishing up Uni. I need to figure out the direction I want rust-http to go in to best meet the requirements of the various classes of users, including (but certainly not limited to!) Servo; thus far the work that has been done has been very largely experimental; I think it's time for a bit of deliberate (rather than accidental) design before progressing much further; I think rust-http will change a fair bit in the process. If these two will be of immediate value to people, I'm open to merging them as they are, but don't expect things to stay the same; quite a lot of rust-http is likely to be rewritten.
(I've looked in some detail at Spray's design; it's the closest thing extant to what I wish to achieve, though there are still very distinct differences in what we need. As with everything other such project, it's inspiration only, not a copying exercise. Rust needs to end up with the best HTTP libraries for just about every type of task, which is a rather large undertaking!)
@chris-morgan Hey, no worries! Thanks for reaching back.
I share your thoughts w.r.t making rust-http the best http library for every task. In my opinion, the API has to expose different layers of abstraction:
- Lower level
- Higher level (Basically the one I worked on)
- Higher level and concurrent
It is important to keep them separate and not hide any of those layers from users. So, since this is already a start and the work has been done, I'd say we should merge it and perhaps use it as a reference for the future refactor process.
That being said, I'm willing to help with the new design so, if you agree, we could work on this together.
Something that bothers me at the moment is that RequestWriter is trying to be too many things. It does DNS lookups, socket management, will implement some of the HTTP spec and manage headers (like ContentLength). The fact that some things which are notoriously brittle (like DNS resolution) happen in a constructor is very concerning.
I think a good first step would be to separate out Request from RequestWriter. This may help answer some questions such as what to do if the user wrote the headers out without (or with a bad) ContentLength (the answer being: "request writer is a low level API and if you call it directly we provide no guarantees you're following the protocol").
@marcbowes I had the same thought when I started using rust-http. I think the final API, as mentioned in my previous comment, should be split in 3 different levels. The first allows the user to do anything but if something goes wrong it the user's fault. The second one, gives a bit less control but it gives more guarantees and reliability. Finally, there could also be a concurrent API on top of the previous 2.
The Reques trait you mention, in my proposal, would be part of the second level.
It's also worth considering that some users are going to want to get hooks into the client. For example, you may want do install a hook that injects an HTTP header. We should dog-food on this extensibility early on in the design process. Concrete examples would be ContentLength and UserAgent: both should be installed as part of the request pipeline, rather than some hard-coded entity. Users should be able to remove the default handlers and replace them with their own.
A concrete use-case I have in mind is talking to AWS services (S3 and so forth). To implement this, we'd need to do operations on the request body and headers "just before" sending the request.
Agreed. This patch https://github.com/FlaPer87/rust-http/commit/dadcca035b8e570a171356ff4a53c03326a292cf allows users to hack the request header and still not having to mess with the RequestWriter directly.
Nice :+1:
@chris-morgan: I think it's time for a bit of deliberate [..] design before progressing much further
Do you think it's worthwhile putting together some sort of proposal/comparison?
One other consideration is testing. It would be nice to be able to the client to write to any stream (possibly many at once). This could also be useful in production.
@marcbowes it'd be cool to have some sort of proposal. IIRC @chris-morgan was working on one.
I kinda think we should let this 2 patches land and re-factor on top of them.
Yup. The code-base is still small enough for this not to matter much (not saying your code is bad; just emphasizing that the risk is low :-)).
@chris-morgan It would be nice if we could move forward with this in some way - either close this PR with some kind of clear details on what a preferred API would be, or merge and iterate.
