gaborgsomogyi commented on code in PR #20066:
URL: https://github.com/apache/flink/pull/20066#discussion_r906720670


##########
flink-runtime/src/test/java/org/apache/flink/runtime/security/token/KerberosDelegationTokenManagerITCase.java:
##########
@@ -79,9 +80,8 @@ public void configurationIsNullMustFailFast() {
     }
 
     @Test
-    public void oneProviderThrowsExceptionMustFailFast() {
-        assertThrows(
-                Exception.class,
+    public void oneProviderThrowsExceptionMustFailSilently() {
+        assertDoesNotThrow(

Review Comment:
   This change is questionable but after in-depth consideration this makes 
sense from my perspective. Several tests were failing because the new provider 
was not able to be loaded (`hadoop-hdfs-client` was not on classpath).
   
   I've considered and rejected the following alternatives:
   * Make `security.kerberos.fetch.delegation-token` to false by default -> 
This would be a breaking change even if it is small. If we would like to choose 
an alternative then I would vote on this.
   * Set `security.kerberos.fetch.delegation-token` to false at all the related 
tests -> Huge amount of unnecessary test code modifications for nothing.
   * At all the related tests `hadoop-hdfs-client` dependency can be added -> 
This would be an overkill since these tests are not using DT framework at all.
   * Start the DT framework only if `hadoop-hdfs-client` is on classpath -> 
This would generate pain for users. A good example is when the Hadoop FS 
provider is turned off but the Flink is not starting until it's not on 
classpath.
   
   With this change the end result is that Flink job will start even if one 
provider is not loaded, only a log error message will appear. What we would 
gain is that in tests we don't need to suffer from not properly configured DT 
framework exceptions.



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

Reply via email to