singhpk234 commented on code in PR #3724:
URL: https://github.com/apache/iceberg-python/pull/3724#discussion_r3786314261


##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -600,6 +690,81 @@ def plan_scan(self, identifier: str | Identifier, request: 
PlanTableScanRequest)
 
         return tasks
 
+    def _plan_scan_result(self, identifier: str | Identifier, request: 
PlanTableScanRequest) -> PlannedScanResult:
+        """Plan a table scan and return tasks with optional plan storage 
credentials.
+
+        Handles the full scan planning lifecycle including async polling and 
pagination.
+        """
+        response = self._plan_table_scan(identifier, request)
+
+        if isinstance(response, PlanFailed):
+            error_msg = response.error.message if response.error else "unknown 
error"
+            raise RuntimeError(f"Received status: failed: {error_msg}")
+
+        if isinstance(response, PlanCancelled):
+            raise RuntimeError("Received status: cancelled")
+
+        if isinstance(response, PlanSubmitted):
+            if not response.plan_id:
+                raise ValueError("Async scan planning submitted without 
plan-id")
+            response = self._poll_until_completed(identifier, response.plan_id)
+
+        if not isinstance(response, PlanCompleted):
+            raise RuntimeError(f"Invalid planStatus for response: 
{type(response).__name__}")
+
+        tasks = self._expand_plan_tasks(identifier, response)
+        return PlannedScanResult(
+            tasks=tasks,
+            storage_credentials=list(response.storage_credentials or []),
+            plan_id=response.plan_id,
+        )
+
+    def plan_scan(self, identifier: str | Identifier, request: 
PlanTableScanRequest) -> list[FileScanTask]:
+        """Plan a table scan and return FileScanTasks.
+
+        Handles the full scan planning lifecycle including async polling and 
pagination.
+
+        Args:
+            identifier: Table identifier.
+            request: The scan plan request parameters.
+
+        Returns:
+            List of FileScanTask objects ready for execution.
+
+        Raises:
+            RuntimeError: If planning fails, is cancelled, or returns 
unexpected response.
+            RemotePlanTimeoutError: If async planning does not complete in 
time.
+            ValueError: If a submitted plan is missing plan-id.
+        """
+        return self._plan_scan_result(identifier, request).tasks
+
+    def _file_io_from_plan(
+        self,
+        existing_properties: Properties,
+        storage_credentials: list[StorageCredential],
+        location: str | None = None,
+    ) -> FileIO | None:
+        """Build a scan-scoped FileIO from plan storage credentials.

Review Comment:
   makes sense to me !



##########
mkdocs/docs/configuration.md:
##########
@@ -386,6 +386,10 @@ catalog:
 | snapshot-loading-mode | refs                           | The snapshots to 
return in the body of the metadata. Setting the value to `all` would return the 
full set of snapshots currently valid for the table. Setting the value to 
`refs` would load all snapshots referenced by branches or tags. |
 | `header.X-Iceberg-Access-Delegation` | `vended-credentials` | Signal to the 
server that the client supports delegated access via a comma-separated list of 
access mechanisms. The server may choose to supply access via any or none of 
the requested mechanisms. When using `vended-credentials`, the server provides 
temporary credentials to the client. When using `remote-signing`, the server 
signs requests on behalf of the client. (default: `vended-credentials`) |
 | view-endpoints-supported | false                           | For backwards 
compatibility with older REST servers. Set to `true` if the server supports 
view endpoints but doesn't send the `endpoints` field in the ConfigResponse. |
+| scan-planning-mode | client | When set to `server`, and the catalog 
advertises the plan-table-scan endpoint, `table.scan()` uses REST server-side 
scan planning. Async plans (`status=submitted`) are polled via `GET 
.../plan/{plan-id}` until completion. |

Review Comment:
   That makes sense, but i think we should also add support for respecting the 
LoadTableResponse config override, because it may happen that a server just 
wants to support scan planning only for certain tables and not blanket make all 
tables do remote scan planning 



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to