rdblue commented on code in PR #15595: URL: https://github.com/apache/iceberg/pull/15595#discussion_r2933653155
########## core/src/main/java/org/apache/iceberg/rest/RESTScanContext.java: ########## @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest; + +import java.util.List; +import java.util.function.BiFunction; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.io.FileIO; +import org.apache.iceberg.rest.credentials.Credential; + +/** Encapsulates REST scan planning context passed from the catalog to the scan. */ +class RESTScanContext { + private final ResourcePaths resourcePaths; + private final TableIdentifier tableIdentifier; + private final boolean supportsAsync; + private final boolean supportsCancel; + private final boolean supportsFetchTasks; + private final BiFunction<List<Credential>, String, FileIO> fileIOFactory; + + RESTScanContext( + ResourcePaths resourcePaths, + TableIdentifier tableIdentifier, + boolean supportsAsync, + boolean supportsCancel, + boolean supportsFetchTasks, Review Comment: I appreciate that you're turning around these updates so quickly! I was thinking about this a little differently, so I'm sorry that I should have been a bit more clear with my suggestion. What I was thinking is a class that behaves like `ResourcePaths`, but at the table level to encapsulate logic that doesn't need to be spread out. Something like a `TableResource`, which would be constructed from `ResourcePaths`, `TableIdentifier`, and `Set<Endpoint>` for table-specific endpoint checks like `supportsAsync`. * `tableResource.planPath()` would return the planning submit path * `tableResource.planPath(planId)` would return the async endpoint to poll for the give plan ID * `tableResource.fetchPath()` would return the endpoint for fetching plan task results I think of this as a "table resource" because it could handle more than just scan planning. For instance, the code for endpoint checks is also littered throughout `RESTTableOperations`. I would want `RESTTableOperations` to also delegate as much as possible to the table resource object as well, rather than keeping a copy of the supported endpoints. This wouldn't need to happen right now, but it's how I think about what I'm proposing. Does that make sense? Do you think it would be a good direction? -- 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]
