XN137 commented on code in PR #2304:
URL: https://github.com/apache/polaris/pull/2304#discussion_r2265765986
##########
runtime/service/src/main/java/org/apache/polaris/service/task/BatchFileCleanupTaskHandler.java:
##########
@@ -53,7 +53,7 @@ public boolean handleTask(TaskEntity task, CallContext
callContext) {
BatchFileCleanupTask cleanupTask =
task.readData(BatchFileCleanupTask.class);
TableIdentifier tableId = cleanupTask.tableId();
List<String> batchFiles = cleanupTask.batchFiles();
- try (FileIO authorizedFileIO = fileIOSupplier.apply(task, callContext)) {
+ try (FileIO authorizedFileIO = fileIOSupplier.apply(task, tableId,
callContext)) {
Review Comment:
> Task handling code should extract the TASK_DATA with readData, which
should contain a TableIdentifier (possibly via a TableLikeEntity) if the
handling code needs a TableIdentifier.
this is already happening: all callers of `TaskFileIOSupplier.apply` are
task handlers and they already get the `TableIdentifier` via
`TaskEntity.readData`. with this PR they are simply passing that
`TableIdentifier` into the `TaskFileIOSupplier`.
> The TaskFileIOSupplier type should not require modification.
the `TaskFileIOSupplier` is what is requiring a `TableIdentifer` for logging
purposes (as its a parameter for `FileIOFactory.loadFileIO`).
without this PR task handlers are simply logging the wrong value because it
treats a `TaskEntity` as a `IcebergTableLikeEntity` and generates a wrong
`TableIdentifier` (pointing to the task entity, instead of the table).
> Yes, that's true. What's the question here?
I am trying to explain to you that what you are asking for is already
happening i.e. tasks already store the `TableIdentifier` in the `TASK_DATA`
property of the `TaskEntity`.
--
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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
