141 add futures #148
141 add futures #148
Conversation
garious
left a comment
There was a problem hiding this comment.
This is definitely what I had in mind - thanks for taking this on! This is an important step in moving to an async client. Just a few nits (comments inline). Also, can you run rustfmt on this code? The CI job will fail otherwise.
| true | ||
| } | ||
|
|
||
| // fn forget_signature_with_last_id(&self, last_id: &Hash) -> bool { |
There was a problem hiding this comment.
Don't need this. Even if not commented out, it's not right. It should remove last_id, not the last item.
| /// until the server sends a response. If the response packet is dropped | ||
| /// by the network, this method will hang indefinitely. | ||
| pub fn get_balance(&self, pubkey: &PublicKey) -> io::Result<Option<i64>> { | ||
| pub fn get_balance(&self, pubkey: &PublicKey) -> FutureResult<i64, i64> { |
There was a problem hiding this comment.
How about something likeFutureResult<Option<i64>, io::Error> ? Or perhaps the library offers a futures::io::FutureResult<Option<i64>>?
| let req = Request::GetBalance { key: *pubkey }; | ||
| let data = serialize(&req).expect("serialize GetBalance"); | ||
| self.socket.send_to(&data, &self.addr)?; | ||
| self.socket.send_to(&data, &self.addr).expect("buffer error"); |
There was a problem hiding this comment.
Can't panic on this one. It's too likely to happen.
| /// it also has the side-effect of causing the server to log any | ||
| /// entries that have been published by the Historian. | ||
| pub fn get_last_id(&self) -> io::Result<Hash> { | ||
| pub fn get_last_id(&self) -> FutureResult<Hash, ()> { |
There was a problem hiding this comment.
Again, io::Errror instead of ()?
|
sure thing, i'll knock this out in a few hours |
|
See #150 |
No description provided.