-
Notifications
You must be signed in to change notification settings - Fork 188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
thread pool of 1 if prefixes is 0, avoid IllegalArgument #743
Conversation
@@ -238,7 +238,7 @@ public SingularityS3Log call() throws Exception { | |||
} | |||
|
|||
private List<SingularityS3Log> getS3Logs(Optional<String> group, Collection<String> prefixes) throws InterruptedException, ExecutionException, TimeoutException { | |||
ListeningExecutorService executorService = MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(Math.min(prefixes.size(), configuration.get().getMaxS3Threads()), | |||
ListeningExecutorService executorService = MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(Math.min(prefixes.size() == 0 ? 1 : prefixes.size(), configuration.get().getMaxS3Threads()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk why i suggested that route -- can't we just short-circuit to Collections.emptyList()
if there are no prefixes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you and your logical ideas... 😜 I'll update
@@ -238,6 +238,10 @@ public SingularityS3Log call() throws Exception { | |||
} | |||
|
|||
private List<SingularityS3Log> getS3Logs(Optional<String> group, Collection<String> prefixes) throws InterruptedException, ExecutionException, TimeoutException { | |||
if (prefixes.size() == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isEmpty() is preferable I believe.
thread pool of 1 if prefixes is 0, avoid IllegalArgument
@tpetr