Clustering #145
Clustering #145
Conversation
…search. Time to make this look good.
…search. Time to make this look good.
| } | ||
|
|
||
| // Should I do this? with the tokio::run |
There was a problem hiding this comment.
I don't think that this matters at all in tests, maybe it's better to keep a standard by using tokio::run everywhere.
| res.into_inner() | ||
| }) | ||
| .map_err(|e| { | ||
| info!("ERR = {:?}", e); |
There was a problem hiding this comment.
prob want this to be an error! 😄
|
|
||
| Box::new(fut) | ||
| Box::new(future::join_all(fut)) |
There was a problem hiding this comment.
This actually has a concrete type so we don't have to box here but I have some ideas around this and its not a big deal right now but down the road we want to remove all these boxes.
There was a problem hiding this comment.
so you'd want the return type to be a JoinAll ?
There was a problem hiding this comment.
We can, that would remove the need for a Box call. though may be harder than that if we use the combinators, though I wouldnt worry about this now. I have some ideas how we can improve this in the future.
| fn list_indexes(&mut self, _: Request<ListRequest>) -> Self::ListIndexesFuture { | ||
| if let Ok(ref mut cat) = self.catalog.read() { | ||
| if let Ok(ref cat) = self.catalog.read() { |
There was a problem hiding this comment.
does this call block? if so we should probably wrap this call in a blocking https://docs.rs/tokio-threadpool/0.1.11/tokio_threadpool/fn.blocking.html
| let result = Some(RpcServer::create_result(0, "".into())); | ||
| let resp = Response::new(RpcServer::create_search_reply(result, query_bytes)); | ||
| future::finished(resp) | ||
| if let Ok(ref cat) = self.catalog.read() { |
|
|
||
| fn place_index(&mut self, request: Request<PlaceRequest>) -> Self::PlaceIndexFuture { | ||
| let PlaceRequest { index, schema } = request.into_inner(); | ||
| if let Ok(ref mut cat) = self.catalog.write() { |
| } | ||
|
|
||
| // Should I do this? with the tokio::run |
There was a problem hiding this comment.
You probably want to use Runtime::block_on one of the things I have been meaning to add to tokio is a tokio_test::block_on that should make this a bit easier.
| } | ||
|
|
||
| pub fn remote_exists(&self, index: &str) -> bool { | ||
| self.get_remote_collection().lock().unwrap().contains_key(index) |
There was a problem hiding this comment.
probably want to use lock()? like you do below
| dbg!(&search); | ||
| self.get_index(index) | ||
| .and_then(move |hand| hand.search_index(search).map(|r| vec![r])) | ||
| .into_future() |
There was a problem hiding this comment.
I believe you don't need this into future here
|
|
||
| tokio::run(s); | ||
| rt.block_on(s); |
There was a problem hiding this comment.
yeah this is what tokio_test::block_on will do pretty much. though I think without the shutdown_on_idle but shutdown_now.
No description provided.