You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@livy.apache.org by GitBox <gi...@apache.org> on 2020/01/25 02:11:17 UTC

[GitHub] [incubator-livy] bersprockets commented on a change in pull request #275: [LIVY-745] Ensure that a single LivyClientFactory gets loaded.

bersprockets commented on a change in pull request #275: [LIVY-745] Ensure that a single LivyClientFactory gets loaded.
URL: https://github.com/apache/incubator-livy/pull/275#discussion_r370907127
 
 

 ##########
 File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
 ##########
 @@ -118,24 +121,21 @@ public LivyClient build() {
     }
 
     LivyClient client = null;
-    ServiceLoader<LivyClientFactory> loader = ServiceLoader.load(LivyClientFactory.class,
-      classLoader());
-    if (!loader.iterator().hasNext()) {
-      throw new IllegalStateException("No LivyClientFactory implementation was found.");
 
 Review comment:
   You could also simply set a flag in the new for loop that iterates over factories. Immediately after the loop, if the flag is not set, you can throw an exception ("No LivyClientFactory implementation was found."). That *might* be cheaper than first creating an iterator and calling hasNext... but I don't know that.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services