mcvsubbu commented on code in PR #8465:
URL: https://github.com/apache/pinot/pull/8465#discussion_r843277312
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java:
##########
@@ -346,6 +347,29 @@ public StringResultResponse getTaskStateDeprecated(
}
}
+ @GET
+ @Path("/tasks/all")
+ @Authenticate(AccessType.READ)
+ @ApiOperation("Read all tasks status")
+ public Map<String, TaskState> getAllTasks() {
+ Map<String, TaskState> taskStates = new HashMap<>();
+ _pinotHelixTaskResourceManager.getTaskTypes()
+ .forEach(taskType ->
taskStates.putAll(_pinotHelixTaskResourceManager.getTaskStates(taskType)));
+ return taskStates;
+ }
+
+ @POST
+ @Path("/tasks/create")
+ @Authenticate(AccessType.CREATE)
Review Comment:
Instead of taking a generic map, can we add specific arguments here? The
arguments can be self-documented instead of the user learning one argument at a
time based on the exception thrown.
Or, maybe this can be a table API (since we always want to run tasks on a
particular table)
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java:
##########
@@ -346,6 +347,29 @@ public StringResultResponse getTaskStateDeprecated(
}
}
+ @GET
+ @Path("/tasks/all")
+ @Authenticate(AccessType.READ)
+ @ApiOperation("Read all tasks status")
+ public Map<String, TaskState> getAllTasks() {
+ Map<String, TaskState> taskStates = new HashMap<>();
+ _pinotHelixTaskResourceManager.getTaskTypes()
+ .forEach(taskType ->
taskStates.putAll(_pinotHelixTaskResourceManager.getTaskStates(taskType)));
+ return taskStates;
+ }
+
+ @POST
+ @Path("/tasks/create")
Review Comment:
I think what you want to do is to {{run}} a specific task type that has
already been defined. Correct? If so, can we change the API to call it "run"
(or "execute")?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java:
##########
@@ -346,6 +347,29 @@ public StringResultResponse getTaskStateDeprecated(
}
}
+ @GET
+ @Path("/tasks/all")
Review Comment:
Can we do that in the client? (i.e. fetch all tasks and then iterate through
the types to get status)?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java:
##########
@@ -128,6 +132,53 @@ public PinotTaskManager(PinotHelixTaskResourceManager
helixTaskResourceManager,
}
}
+ public String createTask(Map<String, String> taskConfigs)
+ throws Exception {
+ String taskName = taskConfigs.get("taskName");
+ if (taskName == null) {
+ throw new IllegalArgumentException("Missing field 'taskName'");
+ }
+ String taskType = taskConfigs.get("taskType");
+ if (taskType == null) {
+ throw new IllegalArgumentException("Missing field 'taskType'");
+ }
+ String tableName = taskConfigs.get("tableName");
+ if (tableName == null) {
+ throw new IllegalArgumentException("Missing field 'tableName'");
+ }
+ String parentTaskName =
_helixTaskResourceManager.getParentTaskName(taskType, taskName);
+ TaskState taskState =
_helixTaskResourceManager.getTaskState(parentTaskName);
+ if (taskState != null) {
+ throw new RuntimeException(
+ "Task [" + taskName + "] of type [" + taskType + "] is already
created. Current state is " + taskState);
+ }
+ if (TableNameBuilder.isRealtimeTableResource(tableName)) {
Review Comment:
Why this limitation ? Let the task decide if it is able to work on realtime
or not
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/FileIngestionHelper.java:
##########
@@ -176,7 +176,7 @@ public static void copyURIToLocal(Map<String, String>
batchConfigMap, URI source
if (!PinotFSFactory.isSchemeSupported(sourceFileURIScheme)) {
PinotFSFactory.register(sourceFileURIScheme,
batchConfigMap.get(BatchConfigProperties.INPUT_FS_CLASS),
IngestionConfigUtils.getInputFsProps(IngestionConfigUtils.getConfigMapWithPrefix(
- batchConfigMap, BatchConfigProperties.INPUT_FS_PROP_PREFIX)));
+ batchConfigMap, BatchConfigProperties.INPUT_FS_PROP_PREFIX +
IngestionConfigUtils.DOT_SEPARATOR)));
Review Comment:
How is this related to adding an adhoc task api? It is better to keep bug
fixes independent.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
