vigneshio commented on code in PR #4962:
URL: https://github.com/apache/polaris/pull/4962#discussion_r3598634593
##########
runtime/service/src/main/java/org/apache/polaris/service/task/TaskExecutorImpl.java:
##########
@@ -229,35 +230,33 @@ protected void handleTask(
.addKeyValue("taskEntityId", taskEntityId)
.addKeyValue("taskType", task.getTaskType())
.log("Unable to find handler for task type");
- throw new RuntimeException(
+ throw new TaskHandlerNotFoundException(
"Unable to find handler for task type "
+ task.getTaskType()
+ " for task entity id "
+ taskEntityId);
}
TaskHandler handler = handlerOpt.get();
- success = handler.handleTask(task, ctx);
- if (success) {
- LOGGER
- .atInfo()
- .addKeyValue("taskEntityId", taskEntityId)
- .addKeyValue("handlerClass", handler.getClass())
- .log("Task successfully handled");
- metaStoreManager.dropEntityIfExists(
- ctx.getPolarisCallContext(), null, taskEntity, Map.of(), false);
- } else {
- LOGGER
- .atWarn()
- .addKeyValue("taskEntityId", taskEntityId)
- .addKeyValue("taskEntityName", taskEntity.getName())
- .log("Unable to execute async task");
- throw new RuntimeException(
- "Task handler returned false for task entity id "
- + taskEntityId
- + " (handler: "
- + handler.getClass().getSimpleName()
- + ")");
- }
+ handler.handleTask(task, ctx);
+ success = true;
+ LOGGER
+ .atInfo()
+ .addKeyValue("taskEntityId", taskEntityId)
+ .addKeyValue("handlerClass", handler.getClass())
+ .log("Task successfully handled");
+ metaStoreManager.dropEntityIfExists(
+ ctx.getPolarisCallContext(), null, taskEntity, Map.of(), false);
+ } catch (TaskHandlerNotFoundException e) {
+ success = false;
+ throw e;
+ } catch (Exception e) {
+ LOGGER
+ .atWarn()
+ .addKeyValue("taskEntityId", taskEntityId)
+ .addKeyValue("taskEntityName", taskEntity != null ?
taskEntity.getName() : "")
+ .log("Unable to execute async task");
+ success = false;
Review Comment:
> Yep, I think that would be a reasonable solution. And as @dimas-b pointed
out, this is not new to this PR.
Sounds good - I will file a follow-up for the sweeper once this is merged.
--
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]
