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

feat(hyper): switch to std::io, std::net, and std::path. #354

Merged
merged 1 commit into from
Mar 3, 2015

Conversation

seanmonstar
Copy link
Member

All instances of old_io and old_path were switched to use the new
shiny std::io, std::net, and std::path modules. This means that
Request and Response implement Read and Write now.

Because of the changes to TcpListener, this also takes the opportunity
to correct the method usage of Server. As with other
languages/frameworks, the server is first created with a handler, and
then a host/port is passed to a listen method. This reverses what
Server used to do.

Closes #347

BREAKING CHANGE: Check the docs. Everything was touched.

@reem
Copy link
Contributor

reem commented Mar 1, 2015

Needs a rebase already.

@@ -21,3 +21,6 @@ time = "*"
unicase = "*"
unsafe-any = "*"
url = "*"

[profile.bench]
opt-level = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

??

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh dangit. I had set that when I was fixing the bench marks, to compile faster.

@seanmonstar seanmonstar force-pushed the new-io branch 2 times, most recently from 6bbfb0d to 2ce4385 Compare March 1, 2015 05:03
@seanmonstar
Copy link
Member Author

@reem rebased.

Interesting to note: speed improvements with the client_mock_tcp bench is over 500%.

BEFORE
test bench_mock_hyper ... bench:    280059 ns/iter (+/- 62609)

AFTER
test bench_mock_hyper ... bench:     50152 ns/iter (+/- 5771)

match *self {
SizedReader(ref mut body, ref mut remaining) => {
debug!("Sized read, remaining={:?}", remaining);
if *remaining == 0 {
Err(old_io::standard_error(old_io::EndOfFile))
Ok(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think Ok(0) is still bad behavior, unless the recent RFC changed that.

Copy link
Member Author

Choose a reason for hiding this comment

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

My understanding is that Ok(0) is EOF. There no longer is an EOF error kind...

Copy link
Contributor

Choose a reason for hiding this comment

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

Asked about this and you're right. (deleted the other comment on this same thing)

@pyfisch pyfisch mentioned this pull request Mar 2, 2015
@seanmonstar
Copy link
Member Author

If there's no real problems, I'd like to merge so we can unbreak people on crates.io...

@akoshochrein
Copy link

Hey, I am not sure if this is a blocker, but with the current build the example client does not build for me. Here's the exception:

src/main.rs:19:20: 19:36 error: type `hyper::client::response::Response` does not implement any method in scope named `read_to_string`
src/main.rs:19     let body = res.read_to_string().unwrap();
                                  ^~~~~~~~~~~~~~~~
src/main.rs:19:36: 19:36 help: methods from traits can only be called if the trait is in scope; the following trait is implemented but not in scope, perhaps add a `use` for it:
src/main.rs:19:36: 19:36 help: candidate #1: use `std::io::Read`
error: aborting due to previous error

@mhristache
Copy link

@akoskaaa read_to_string() was changed in the new io module. Looks like the example needs to be updated. Here is how I modified my code to make it compile:

    use std::io::Read;
    ...
    let mut body: String = "".to_string();
    try!(res.read_to_string(&mut body));

All instances of `old_io` and `old_path` were switched to use the new
shiny `std::io`, `std::net`, and `std::path` modules. This means that
`Request` and `Response` implement `Read` and `Write` now.

Because of the changes to `TcpListener`, this also takes the opportunity
to correct the method usage of `Server`. As with other
languages/frameworks, the server is first created with a handler, and
then a host/port is passed to a `listen` method. This reverses what
`Server` used to do.

Closes #347

BREAKING CHANGE: Check the docs. Everything was touched.
@seanmonstar
Copy link
Member Author

Alright, I updated the REAME to work, and renamed that odd function. Once travis hits green, I'll merge. (sorry those using git rev's, I probably broke you because of my force-push)

seanmonstar added a commit that referenced this pull request Mar 3, 2015
feat(hyper): switch to std::io, std::net, and std::path.
@seanmonstar seanmonstar merged commit 9e07708 into master Mar 3, 2015
@seanmonstar seanmonstar deleted the new-io branch March 3, 2015 23:11
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.

Switch to new std::io
4 participants