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



##########
File path: 
extensions-core/azure-extensions/src/test/java/org/apache/druid/data/input/azure/AzureInputSourceTest.java
##########
@@ -214,6 +216,78 @@ public void test_toString_returnsExpectedString()
     Assert.assertEquals("AzureInputSource{uris=[], 
prefixes=[azure://container/blob], objects=[]}", actualToString);
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void test_Deny_All()

Review comment:
       Is this the same as the `testDenyAll()` test below?

##########
File path: 
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamSamplerSpec.java
##########
@@ -102,7 +106,7 @@ public SamplerResponse sample()
           "[spec.ioConfig.inputFormat] is required"
       );
     }
-
+    inputSource.validateAllowDenyPrefixList(securityConfig);

Review comment:
       nit: this doesn't look harm because neither `RecordSupplierInputSource` 
nor `FirehoseFactoryToInputSourceAdaptor` do anything in 
`validateAllowDenyPrefixList()`. However, it looks confusing since 
`validateAllowDenyPrefixList()` is not called in `RecordSupplier`. I would 
suggest to delete this line and add it back when we truly support this security 
config for stream ingestion.




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