rangareddy opened a new pull request, #946:
URL: https://github.com/apache/incubator-xtable/pull/946
Draft implementation for #497. **Design review is in #945 (RFC-4); this is
the code.** Raised so
the RFC can be checked against something runnable. Not proposed for merge as
it stands, and it
should not land before the RFC has direction.
## What it implements
`spec/rest-service-open-api.yaml` already declares an async contract that
nothing implements:
`Prefer: respond-async` returning `202` with a `conversion-id`, and
`GET /v1/conversion/table/{conversion-id}`. Grepping
`xtable-service/src/main/java` for `Prefer`,
`respond-async` or `conversion-id` returns nothing. This implements that
published contract rather
than inventing a parallel `/jobs` API.
| Endpoint | Status |
| --- | --- |
| `POST /v1/conversion/table` with `Prefer: respond-async` | New, already in
the spec |
| `GET /v1/conversion/table/{conversion-id}` | New, already in the spec |
| `GET /v1/conversion/runs` | New |
| `GET /v1/conversion/runs/{id}` | New |
| `GET /v1/conversion/runs/{id}/events?after=N` | New |
Without the `Prefer` header the endpoint behaves exactly as it does today. A
regression test pins
that, because it is released behaviour.
Run history is in memory and bounded, evicting oldest-first, and the UI
presents it as a view of
the running service rather than an audit log.
The UI is plain HTML, CSS and JavaScript with no npm or Node build step. It
reflects what the
service can actually do: Parquet and Paimon are shown disabled because
`ConversionService`
registers providers for Hudi, Delta and Iceberg only; there is no
full-versus-incremental control
because `ConversionController` decides that from the target's
`TableSyncMetadata`; the
partition-spec field appears only for a Hudi source.
## Security gate
`xtable-service/README.md` says the service "ships with no authentication
and no authorization".
The UI is served through a JAX-RS resource gated by `xtable.ui.enabled`,
which **defaults to
false**, so an existing deployment does not gain a browser interface by
upgrading. Assets
deliberately do not live in `META-INF/resources`, which Quarkus would serve
automatically, so the
flag is a real gate rather than an advisory setting. Only `html`, `css` and
`js` are servable.
Whether this is sufficient is the open question in #945.
## A pre-existing bug this turned up
**`xtable-service` cannot perform any conversion when it is actually run**,
on `main` and on the
released 0.4.0 line. Calling the released synchronous endpoint on unmodified
code:
```
POST /v1/conversion/table -> HTTP 500
java.util.ServiceConfigurationError:
org.apache.xtable.spi.sync.ConversionTarget:
org.apache.xtable.kernel.DeltaKernelConversionTarget Unable to get public
no-arg constructor
Caused by: java.lang.ClassNotFoundException: io.delta.kernel.engine.Engine
```
`xtable-core` registers four `ConversionTarget` implementations through
`META-INF/services` and
`ServiceLoader` instantiates the list eagerly. `DeltaKernelConversionTarget`
imports
`io.delta.kernel.defaults.engine.DefaultEngine` from
`delta-kernel-defaults`, which
`xtable-service` does not declare at all; `delta-core` and
`delta-kernel-api` are declared but at
`test` scope. One bad provider aborts the whole iteration, so even an
Iceberg-only conversion
fails. `ITConversionService` does not catch it because integration tests run
with test-scope
artifacts on the classpath.
The fix is the first commit here and is kept separate on purpose: it is not
part of the UI work,
it fixes the released synchronous endpoint too, and it probably deserves its
own PR and a backport
to `branch-0.4`. Happy to split it out. `xtable-service` does not use
`maven-shade-plugin`, so no
shade include list or bundled license metadata is affected.
## Verification
Run against a real Hudi table with `./mvnw quarkus:dev -pl xtable-service`
on JDK 11:
```
POST /v1/conversion/table (Prefer: respond-async) -> 202 + conversion-id
GET /v1/conversion/table/{id} while running -> 202
GET /v1/conversion/table/{id} once finished -> 200, SUCCEEDED in
9134ms
-> ICEBERG file:/tmp/hudi-dataset/people/metadata/v3.metadata.json
-> DELTA file:/tmp/hudi-dataset/people/_delta_log
events: accepted -> started -> wrote ICEBERG -> wrote DELTA -> succeeded
```
A conversion against a missing source table finishes as `FAILED` with the
Hudi error surfaced on
the run rather than hanging.
That last point was a bug in the first version of this code, found by
running it. A missing engine
arrives as `ServiceConfigurationError`, an `Error` and not an `Exception`,
so the worker died
silently and the run sat at `RUNNING` forever while the UI polled it. The
worker now catches
`Throwable`, because it owns the whole lifecycle of a run. There is a
regression test.
23 unit tests pass in `xtable-service`, `spotless:check` is clean, and
`apache-rat:check` is clean
across all modules.
## Two places this diverges from the RFC as first written
Both are recorded in #945 with the reasoning.
1. **Progress events instead of captured logs.** Quarkus routes logging
through the JBoss
LogManager while `xtable-core` logs through Log4j2, so a per-run Log4j2
appender needs a
logging bridge this module does not declare.
2. **Cursor polling instead of SSE.** `?after=<sequence>` needs no new
dependency and works with
the existing blocking JAX-RS setup. SSE over Mutiny `Multi` stays the
better end state, and the
cursor endpoint shape does not have to change when it arrives.
## Not built
Phases 4 and 5 of the RFC: the validation endpoint and table detail screen,
and catalog sync.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
