feat(scheduler): document REST API with OpenAPI and serve /api/openapi.json - #2397
Conversation
|
would you mind having a look @avantgardnerio ? |
e4dd39c to
243937f
Compare
andygrove
left a comment
There was a problem hiding this comment.
Thanks for picking this up. A machine-readable spec for the REST API is a genuinely useful thing to have, and the cfg_attr gating on ballista-api-types is the right call so downstream users of that crate see no change by default.
I left a few inline comments. The one about ExecutorOperatingSystemSpecificationSchema is the one I would like sorted before this lands.
One thing that does not fit on a line in the diff: docs/source/user-guide/scheduler.md has the table of REST endpoints around line 78. Could you add /api/openapi.json to it?
| /// Operating system level specification of an executor. | ||
| #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, utoipa::ToSchema)] | ||
| #[schema(as = ExecutorOperatingSystemSpecification)] | ||
| pub struct ExecutorOperatingSystemSpecificationSchema { |
There was a problem hiding this comment.
This mirror struct declares three fields, but the real ExecutorOperatingSystemSpecification in ballista/core/src/serde/scheduler/mod.rs has nine. It also carries os_ver_long, physical_cores, num_disks, total_disk_space, total_available_disk_space and open_files_limit, and all of them get serialized.
Since #[schema(value_type = ...)] swaps the schema out wholesale, the published spec will under-describe what /api/executors and /api/executor/{executor_id} actually return, right from the first release. ExecutorSpecificationSchema above happens to match today, but it is the same hand-copied pattern and will drift the same way as soon as someone edits the core type.
Would it work to derive ToSchema on the real types in ballista-core behind an optional feature, the same way you have done it for ballista-api-types? That way there is only one definition to keep right.
If you would rather keep this PR small, filling in the six missing fields would fix the immediate problem. In that case it might be worth adding a test that asserts the schema's property set matches the JSON keys of ExecutorOperatingSystemSpecification::default(), so the next drift shows up as a failing test rather than a quietly wrong spec.
| struct GraphvizApiDoc; | ||
|
|
||
| /// Generate the OpenAPI specification for the scheduler REST API. | ||
| pub fn openapi_spec() -> utoipa::openapi::OpenApi { |
There was a problem hiding this comment.
This rebuilds the whole document on every request to /api/openapi.json. The spec is fixed at compile time, so wrapping it in a std::sync::LazyLock would turn the handler into a cheap clone of a cached value.
| crate::api::handlers::get_job_dot_graph, | ||
| crate::api::handlers::get_query_stage_dot_graph, | ||
| crate::api::handlers::get_scheduler_metrics, | ||
| crate::api::openapi::get_openapi_spec, |
There was a problem hiding this comment.
/healthz and /readyz are missing here. They are mounted by health_routes unconditionally, so they are live even when rest-api is off, and they are exactly the two endpoints a platform owner wires into Kubernetes probes.
Anyone generating a client or a monitoring config from openapi.json will not see them. Happy for this to be a follow-up if you would rather keep Phase 1 focused.
| use utoipa::ToSchema; | ||
|
|
||
| #[test] | ||
| fn test_dto_schemas() { |
There was a problem hiding this comment.
This one is mostly asserting that the derive macro does what it says, so I am not sure it earns its keep. A check that the generated schema's properties line up with the actual serde output would catch a lot more, things like a skip_serializing_if that the schema does not know about.
Worth knowing too that this only runs when feature unification turns the utoipa feature on from the scheduler side. cargo test -p ballista-api-types on its own will skip it silently.
|
Thank you for the review and suggestions @andygrove! I've addressed all feedback in the latest commit:
|
63fd4b2 to
db36492
Compare
avantgardnerio
left a comment
There was a problem hiding this comment.
I see you addressed @andygrove 's feedback, so ✔️ with one nice-to-have in the comments.
db36492 to
19c79f7
Compare
|
Can you please rebase pr @DebadityaHait |
19c79f7 to
87c8cd4
Compare
|
Rebased onto latest |
|
thanks @DebadityaHait, @avantgardnerio & @andygrove will merge this on green ci |
Merging main brought in the OpenAPI endpoint (apache#2397) and build-side staging for AQE joins (apache#2434). Neither was reflected in the docs this branch had already made complete, and auditing them turned up two older inaccuracies in the same sections. From the merged changes: - Document the new `utoipa` feature on ballista-core in the root and crate READMEs, and note that the scheduler's `rest-api` feature now also serves the OpenAPI spec. - Add build-side staging to the AQE optimization lists in the tuning and architecture guides, and to the 55.0.0 upgrade guide, since it is a new default that changes query plans. Pre-existing inaccuracies: - The tuning guide's Join Strategy section only described the static planner. With AQE on by default, `prefer_hash_join` is not consulted; the join is chosen at runtime from the broadcast threshold and `hash_join_max_build_partition_bytes`. The section now covers both paths, and notes that the static planner only promotes hash joins to broadcast. - Join reordering compares byte sizes first and falls back to row counts, not row counts alone. - The REST API was described as an optional feature to enable, but `rest-api` is on by default.
* docs: correct stale defaults, flags, and feature docs AQE was documented as experimental and off by default; it is on by default. The scheduler policy default is push-staged and the event-loop buffer is 1000. ballista-cli has no spark-compat feature. The REST table was missing endpoints and both health probes, the Prometheus list named a metric that is not exported, and every second-executor example moved only --bind-port. CLI usage is regenerated from the built binary. Also brings the docs up to date with #2397 (utoipa, rest-api default) and #2434 (build-side staging). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: keep the Python SessionConfig limitation and name AQE join selection consistently A SessionConfig passed as config is still used only by the local context; only cluster_config reaches the cluster, so restore the limitation and point at cluster_config. The tuning guide listed join reordering and broadcast join selection as two optimizations, while the architecture guide calls them one. Both are the same AQE rule, so merge them into a single Join selection bullet, and say that the build side is chosen by byte size, falling back to row count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: fix the Spark-functions CLI claim, sample logs, and benchmark formats - ballista-cli does not get Spark functions from the cluster: SQL is planned in the client, so the CLI needs them compiled in. Say so, and give the source build that enables them (ballista-core's spark-compat, unified in). - Replace sample startup logs with what the current binaries print. The executor sample showed messages that no longer exist, and a pull-mode poll loop although push-staged is the default. - Keep --format tbl in the benchmark commands; tpch reads dbgen's .tbl files. - Note that the published images lack keda-scaler, that --disable-rest-api turns the REST API off at runtime, and tidy the Prometheus note. - Stop quoting rust_clippy.sh's exact command, which #2473 changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: move Who uses Ballista from the README to the docs The README is the project's entry point, so keep the adopters list in the published docs instead, as a Community page. All three of its links were checked and resolve. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Which issue does this PR close?
Closes #2351
Rationale for this change
Ballista's public REST interfaces currently lack a machine-readable OpenAPI specification. As proposed in #2351, this PR addresses Phase 1 by using
utoipato generate OpenAPI documentation directly from our Rust DTO types and serving the spec atGET /api/openapi.json.What changes are included in this PR?
utoipa(5.5) as a workspace dependency withmacrosenabled.utoipafeature toballista-api-typesand derivedutoipa::ToSchemaon shared DTO types (JobResponse,TaskStatus,TaskSummary,Percentiles,QueryStageSummary,QueryStagesResponse,PlanFormat).#[utoipa::path(...)]annotations to all public scheduler endpoints inballista-scheduler(/api/state,/api/version,/api/executors,/api/executor/{executor_id},/api/jobs,/api/job/{job_id},/api/job/{job_id}/config,/api/job/{job_id}/stages,/api/job/{job_id}/dot,/api/job/{job_id}/stage/{stage_id}/dot,/api/metrics, and/api/job/{job_id}/dot_svgwhengraphviz-supportis enabled).ballista_scheduler::api::openapidefining theApiDocstructure andGET /api/openapi.jsonroute handler.ballista_schedulerandballista_api_typesasserting schema registration, path coverage, serialization, and endpoint response.Are there any user-facing changes?
The scheduler REST API now exposes
GET /api/openapi.jsonreturning the OpenAPI v3 JSON specification. No existing endpoints or serialization schemas were modified.