laserninja opened a new pull request, #12412: URL: https://github.com/apache/gravitino/pull/12412
### What changes were proposed in this pull request? This is PR 2 of the stack that splits #12194, as agreed with @nevzheng ([plan](https://github.com/apache/gravitino/pull/12194#issuecomment-5228220890)). PR 1 is #12411. It wires up `POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks`, the second step of the Iceberg REST two-step scan planning protocol, without yet implementing what it hands back: - `IcebergTableOperations.fetchScanTasks` - the JAX-RS resource, with the same authorization expression as `POST .../plan` since it is the same read of the same table. - `fetchScanTasks` through the table dispatcher chain: `IcebergTableOperationDispatcher` (interface), `IcebergTableEventDispatcher` (events), `IcebergTableHookDispatcher` (pass-through, the operation is read-only), `IcebergTableOperationExecutor` (delegation to the catalog wrapper). - `OperationType.FETCH_SCAN_TASKS` and `AuditLog.Operation.FETCH_SCAN_TASKS`, with `IcebergFetchScanTasksPreEvent`, `IcebergFetchScanTasksEvent` and `IcebergFetchScanTasksFailureEvent`. - `NoSuchPlanTaskException` mapped to `404` in `IcebergExceptionMapper`. `CatalogWrapperForREST.planTableScan` still returns every file scan task inline and hands out no `plan-tasks`, so no plan task presented to this endpoint was issued by this server, and `CatalogWrapperForREST.fetchScanTasks` rejects every one of them as unknown. Two deliberate consequences: - **The endpoint is not advertised in `/v1/config`.** Clients gate on the advertised endpoint set rather than probing - pyiceberg enables server-side scan planning when it sees `POST .../tasks` there, and fails the scan if the endpoint then serves nothing. Advertising it before it can return tasks would be worse than not having it. From a client's point of view this PR leaves behaviour exactly as it is today. - **The success path of the endpoint arrives with batching**, in the last PR of the stack, together with `TestIcebergFetchScanTasksEndpoint` which exercises it end to end. I checked that `FetchScanTasksResponse` is already covered by `RESTSerializers.registerAll`, so the response serialization this PR leaves untested is not an open question. One change here is not strictly about `/tasks`: both scan planning endpoints now report `400` instead of `500` when called with no request body. Jersey passes the resource method a `null` entity, which became an NPE downstream. `/tasks` needed the guard, and applying the same three lines to `/plan` seemed better than leaving the neighbouring endpoint wrong. Happy to drop it into its own PR if you would rather keep this one single-purpose. ### Why are the changes needed? #11284 asks for the fetch scan tasks endpoint. Reviewers on #12194 asked for that work in reviewable pieces; this is the plumbing piece, separated from the batching logic that gives it something to return. Fix: #11284 ### Does this PR introduce _any_ user-facing change? A new REST route exists but is not advertised, and answers `404 NoSuchPlanTaskException` for any plan task. No new configuration properties. The only behaviour change a user can observe is that `POST .../plan` and `POST .../tasks` with an empty body now return `400` instead of `500`. ### How was this patch tested? `./gradlew :iceberg:iceberg-rest-server:check -PskipITs` and `./gradlew :core:test --tests "*TestCompatibilityUtils*"`, both green. Three tests added to `TestIcebergTableOperations`, each run against both a flat and a nested namespace: - `testFetchScanTasksUnknownPlanTask` - a plan task this server never issued is a `404`. Asserts on the error payload (`type` is `NoSuchPlanTaskException`, message names the rejected plan task), not just the status, so it cannot pass against an unregistered route. Asserts the pre event and the failure event are dispatched. - `testFetchScanTasksTableNotFound` - a missing table reports `NoSuchTableException`, not a masked unknown plan task. - `testScanPlanningEndpointsRejectMissingRequestBody` - an empty body on `/plan` and on `/tasks` is a `400`. -- 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]
