feat(rest): implement scan planning client methods#1324
Conversation
|
@laskoviymishka Please take a look when you got time. |
zeroshade
left a comment
There was a problem hiding this comment.
A couple of non-blocking documentation nits from the scan-planning client methods (comment-only, not requesting changes; the low-level surface looks spec-compliant otherwise):
Stale file header - catalog/rest/scan_planning.go lines 17-24: the top-of-file comment still says the method bodies are intentionally unimplemented stubs (returning ErrNotImplemented) and that endpoint capability discovery lands separately in the Phase 0 PR. This PR implements PlanTableScan / FetchPlanningResult / CancelPlanning / FetchScanTasks plus the Supports* capability checks, so that header is now stale and worth updating or removing. (Noting it here rather than inline because those lines aren't part of the diff.)
| // cancellation using a detached context with a short timeout. | ||
| // cancellation using a detached context with a short timeout. The spec supports | ||
| // idempotency and access-delegation headers on cancel; this low-level method | ||
| // deliberately defers those until a cancel options type is added, and suppresses |
There was a problem hiding this comment.
nit: per the REST spec, cancelPlanning does accept an Idempotency-Key (the idempotency-key parameter on the DELETE in rest-catalog-open-api.yaml), so deferring it here is reasonable - just flagging so the follow-up that introduces the cancel options type doesn't lose it. The best-effort cancel + 204 handling and access-delegation suppression here look correct.
e0d6dd4 to
653fd5f
Compare
Updated the stale scan_planning.go file header to reflect the implemented low-level client methods and remaining follow-up scope. |
Summary
This PR implements low-level REST client support for server-side scan planning.
It adds scan-planning endpoint constants, capability checks, request/header plumbing, and the four low-level client methods:
PlanTableScanFetchPlanningResultCancelPlanningFetchScanTasksThis is intentionally scoped to the client-method layer only. Higher-level scan orchestration is left for follow-up work.
What Changed
Endpoint Negotiation
Added constants for the scan-planning REST operations:
POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/planGET /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan/{plan-id}DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan/{plan-id}POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasksThese are included in
allEndpointsfor template validation, but deliberately not indefaultEndpoints. Catalogs must explicitly advertise scan-planning support.Capability Checks
Implemented:
SupportsPlanTableScanSupportsFullRemoteScanPlanningSupportsRemoteScanPlanningSupportsFullRemoteScanPlanningrequires all four endpoints because an initial plan may return async or fanout handles that require polling and task fetch support.Request/Header Handling
Added per-request support for:
Idempotency-KeyX-Iceberg-Access-DelegationPlanTableScanalways sends an idempotency key and uses either a per-request access-delegation value or the session default.FetchPlanningResultuses a per-request access-delegation value when provided, otherwise the session default.FetchScanTaskssends an idempotency key and suppresses access delegation.CancelPlanningsuppresses access delegation and intentionally defers cancel-specific idempotency/access-delegation API shape until an options type is added.Shared REST Plumbing
Generalized internal REST helpers so requests can:
sessionTransport.RoundTripnow treats any header already present on a request as overriding the session default. This is needed so scan-planning calls can override or suppress the defaultX-Iceberg-Access-Delegation: vended-credentialsheader.Response Validation
Preserved
PlanTableScanResponse.UnmarshalJSONvalidation.Added validation for
FetchPlanningResultResponse:A failed
PlanTableScanresponse still decodes as(resp, nil)when the failed arm contains an error payload; callers must branch onresp.Status.Out Of Scope
This PR does not implement:
WaitForPlanCatalog.PlanFilesTests
Added focused tests for endpoint rendering, capability checks, endpoint gating, request headers, default/override access delegation behavior, idempotency key behavior, response validation,
ErrPlanExpiredmapping, and request-helper body closing.Validated with: