ARROW-12332: [Rust] [Ballista] Add simple api server in scheduler#9987
ARROW-12332: [Rust] [Ballista] Add simple api server in scheduler#9987msathis wants to merge 3 commits intoapache:masterfrom msathis:master
Conversation
|
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename pull request title in the following format? or See also: |
msathis
changed the title
|
@andygrove Please review when you find time :) |
andygrove
changed the title
|
Hi @msathis and thanks for creating the PR. I pulled your branch and tried testing this locally but couldn't get it to work. I'm probably doing something wrong. It would be good if you could add some documentation as part of this PR. Perhaps provide a |
@andygrove The above curl should work. Added small documentation to the PR as well. |
|
Hi guys, I don't know how many endpoints you plan to have in future, but did you consider having additional prefix in path like |
|
@Cheappie I don't think we need the prefix since the api is part of the scheduler. But my knowledge is limited. @andygrove Do you foresee a requirement to support additional resources not related to schedulers in future? |
andygrove
left a comment
andygrove
left a comment
There was a problem hiding this comment.
LGTM. I tested based on the provided curl command:
curl --request GET --url http://localhost:50050/executors --header 'Accept: application/json'
[{"id":"7b6069c3-b56a-4909-882b-97f103cdecc2","host":"localhost","port":50051}]
|
@andygrove Can you please get it merged, if everything is okay? |
It's a good point but we're so early with this feature it is hard to know how it will evolve. The process is a dedicated scheduler so adding a scheduler prefix seems redundant at this point though. |
| .serve(make_service_fn(move |_| { | ||
| let scheduler_server = SchedulerServer::new(config_backend.clone(), namespace.clone()); | ||
| let scheduler_grpc_server = SchedulerGrpcServer::new(scheduler_server.clone()); | ||
|
|
||
| let mut tonic = TonicServer::builder() | ||
| .add_service(scheduler_grpc_server) | ||
| .into_service(); | ||
| let mut warp = warp::service(get_routes(scheduler_server)); | ||
|
|
||
| future::ok::<_, Infallible>(tower::service_fn( | ||
| move |req: hyper::Request<hyper::Body>| { | ||
| let header = req.headers().get(hyper::header::ACCEPT); | ||
| if header.is_some() && header.unwrap().eq("application/json") { | ||
| return Either::Left( | ||
| warp.call(req) | ||
| .map_ok(|res| res.map(EitherBody::Left)) | ||
| .map_err(Error::from), | ||
| ); | ||
| } | ||
| Either::Right( | ||
| tonic | ||
| .call(req) | ||
| .map_ok(|res| res.map(EitherBody::Right)) | ||
| .map_err(Error::from), | ||
| ) | ||
| }, | ||
| )) |
There was a problem hiding this comment.
Sorry I'm super late to this PR. Does this change mean that a SchedulerGrpcServer is being built for every request?
4 participants
Implements GET /executors. We can additional endpoints going forward.