Reimplement server.rs with warp #20
Conversation
|
Hey, Impressive, I think we will opt for The limit of object length with If SSE (or WebSocket) integration can be facilitated it's good, also I found your code lighter and cleaner. Very nice |
|
This should be done now, please let me know if anything is missing/wrong. There's still more to be done for logging (almost no logging right now), but I'm likely going to do that in another PR because it requires migrating to the log crate. |
| pub bind_address: String, | ||
| pub port: i32, | ||
| pub port_ssl: i32, | ||
| pub bind_address: IpAddr, |
There was a problem hiding this comment.
You changed the type of bind_address, is it not a problem for the YAML configuration file?
There was a problem hiding this comment.
Serde implements Deserialize for IpAddr, and it seems like it just uses the appropriate from_str function internally (or at least it parses the previous configuration correctly).
| store: Arc<RwLock<KvStore>>, | ||
| configuration: Configuration | ||
| } | ||
| let auth = warp::header::optional::<String>("authorization") |
There was a problem hiding this comment.
Could you enable JWT authentication only if it's enabled in the configuration?
There was a problem hiding this comment.
Yes, it should be easy to add a check if it's enabled in verify_auth and accept all requests if it's not.
|
Very good work, I merge it 👌 |
|
@CephalonRho If you need to continue your implementation, feel free to use the new |
|
@CephalonRho think also make a gracefully exit for |
|
Closed #18 |
This should make Lucid significantly faster than when using nickel. The results are pretty impressive, so I guess warp lives up to it's name.
Warp:
Nickel:
It should also make it easier to implement WebSocket or SSE later because warp has built-in support for both.
The API changes very slightly (PUT requests to /api/kv are required to have a Content-Length header), but this should allow warp to reject big requests immediately instead of waiting for them to arrive and is trivial to implement for clients.
This is just a proposal though, and there's still some work to be done to reach parity with the current implementation.