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

Should we add optional async? #64

Closed
steveklabnik opened this issue Oct 16, 2017 · 3 comments
Closed

Should we add optional async? #64

steveklabnik opened this issue Oct 16, 2017 · 3 comments
Labels

Comments

@steveklabnik
Copy link
Owner

We could leverage async in this crate in various ways.

We didn't build this on top of tokio because we wanted to keep the code simple. However, we could optionally add it in one of two ways:

First, the bit that's the same in both of them: add an async Cargo feature that lets you turn async on. That way, without the flag, you get only sync io and little deps, but if you wanted it to be async, you could do so.

Option 1: new method

This option would add an alternate Server, AsyncServer. It would look something like this:

extern crate simple_server;

use simple_server::{AsyncServer, ok};


fn main() {
    let host = "127.0.0.1";
    let port = "7878";

    let server = AsycncServer::new(|request, mut response| {
        ok(response.body("Hello Rust!".as_bytes())?)
    });

    server.listen(host, port);
}

Option 2: add futures

Option two is, change the interface of Server to be futures-enabled generally. The futures crate has no dependencies, and so is still pretty small. Then, the switch would only be in the backend, not in the interface.


The tradeoff is basically, with one, we split the "ecosystem", as it were. Option 2 adds a small amount of complexity for the simple case, but unifies everything into one API.

@steveklabnik
Copy link
Owner Author

Talking with @ashleygwilliams , here's the plan:

  1. Port sparkles to simple-server
  2. port thanks to new sparkles
  3. try out both of these approaches and compare the diffs

this way we can actually see how much of a change this would be

@steveklabnik
Copy link
Owner Author

I took a stab at this when I had some time today and tried out option 2: https://github.com/steveklabnik/simple-server/tree/futures

the ergonomics of using it are really bad, and I think that once ? works for futures, it'll be great.

I also may have just gotten the generic wrong...

@steveklabnik
Copy link
Owner Author

I think the answer to this is "no" for now. maybe someday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant