REST: Add Idempotency-Key support for scan planning endpoints #15096
Conversation
| private String planPath(TableIdentifier ident) { | ||
| return String.format( | ||
| "v1/namespaces/%s/tables/%s/plan", | ||
| RESTUtil.encodeNamespace(ident.namespace(), RESTUtil.NAMESPACE_SEPARATOR_URLENCODED_UTF_8), | ||
| RESTUtil.encodeString(ident.name())); | ||
| } | ||
|
|
||
| private String tasksPath(TableIdentifier ident) { | ||
| return String.format( | ||
| "v1/namespaces/%s/tables/%s/tasks", | ||
| RESTUtil.encodeNamespace(ident.namespace(), RESTUtil.NAMESPACE_SEPARATOR_URLENCODED_UTF_8), | ||
| RESTUtil.encodeString(ident.name())); | ||
| } | ||
|
|
||
| private String cancelPath(TableIdentifier ident, String planId) { | ||
| return String.format( | ||
| "v1/namespaces/%s/tables/%s/plan/%s", | ||
| RESTUtil.encodeNamespace(ident.namespace(), RESTUtil.NAMESPACE_SEPARATOR_URLENCODED_UTF_8), | ||
| RESTUtil.encodeString(ident.name()), | ||
| RESTUtil.encodeString(planId)); | ||
| } |
There was a problem hiding this comment.
| FetchScanTasksResponse.class, | ||
| ErrorHandlers.planTaskHandler()); | ||
|
|
||
| // We cancel the planning state before this call, so a *fresh execution* of fetchScanTasks would |
There was a problem hiding this comment.
| // We cancel the planning state before this call, so a *fresh execution* of fetchScanTasks would | |
| // We cancel the planning state before this call, so a fresh execution of fetchScanTasks would |
??
| List<PlanTableScanResponse> responses = | ||
| executeTwice( | ||
| HTTPRequest.HTTPMethod.POST, | ||
| planPath(ident), | ||
| headers, | ||
| request, | ||
| PlanTableScanResponse.class, | ||
| ErrorHandlers.tableErrorHandler()); | ||
| PlanTableScanResponse first = responses.get(0); | ||
| PlanTableScanResponse second = responses.get(1); |
There was a problem hiding this comment.
i am not sure if execute twice is getting something here we any ways are writing 2 loc to get the first and the second invocation ?
There was a problem hiding this comment.
Agreed executeTwice doesn’t really reduce verbosity here. I’ve updated the test to use two explicit execute(...) calls for first and second to make the two invocations unambiguous
| // identifiers (data file locations) to the first response. Use order-insensitive comparison | ||
| // since task ordering isn't guaranteed. |
There was a problem hiding this comment.
why isn't the task ordering gauranteed ?
There was a problem hiding this comment.
In CatalogHandlers.planFilesFor, we iterate scan.planFiles() and directly Iterables.partition(planTasks, tasksPerPlanTask) without sorting, so task order is whatever planFiles() yields and isn’t guaranteed.
2 participants
Follows OpenAPI spec update in #14730 to support Idempotency-Key on mutating scan planning endpoints.