jihoonson commented on a change in pull request #10677:
URL: https://github.com/apache/druid/pull/10677#discussion_r556275449



##########
File path: 
indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTask.java
##########
@@ -1044,17 +1048,21 @@ public IndexIOConfig(
       if (firehoseFactory != null && inputFormat != null) {
         throw new IAE("Cannot use firehose and inputFormat together. Try using 
inputSource instead of firehose.");
       }
+      if (inputSource != null) {
+        inputSource.validateAllowDenyPrefixList(securityConfig);

Review comment:
       > Just to double check, if i move it to isReady the task will be 
accepted by the overlord and it will fail when the task is actually tried to be 
run. I was thinking that if someone is not allowed, we should block the 
ingestion at that time when the task is submitted at first place. Is that the 
behavior you are expecting ?
   
   @nishantmonu51 `Task.isReady()` is actually called as soon as it is issued 
to the Overlord. When it returns true, the Overlord changes its state from 
`waiting` to `pending`, and tries to assign it to one of middleManagers. So, I 
think it will be effectively the same as what you want to achieve.
   
   > isReady is also called multiple times, so not sure if that is any better 
in terms of number of checks,
   
   Good point. I think it won't be not that expensive, but maybe we can check 
the `validated` state and avoid further checking if it is already validated?




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org

Reply via email to