Skip to content

Conversation

madplay
Copy link

@madplay madplay commented Dec 13, 2024

This PR improves code quality by:

  • Fixing typos, optimizing iteration.
  • Refactoring logic, removing unused parameters.

I found this project while looking into Kafka connectors. It's really impressive, thanks ☺️

@madplay madplay requested review from a team as code owners December 13, 2024 13:59
@@ -115,7 +115,7 @@ public static <T, E extends Exception> T callWithRetry(final String callName, fi
final long sleepTimeMs = computeRandomRetryWaitTimeInMillis(retryAttempts, retryBackoffMs);
final var msg = String.format("Failed to %s with attempt %s/%s, will attempt retry after %s ms. ",
callName, attempts, maxAttempts, sleepTimeMs);
LOGGER.warn(msg + "Failure reason: {}", e);
LOGGER.warn(msg + "Failure reason: ", e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be:

LOGGER.warn("{} Failure reason {}", msg, e.getMessage(), e);

or something similar. Perhaps just

LOGGER.warn(msg,  e);

Comment on lines 35 to +39
final Collection<OpensearchClientConfigurator> configurators = new ArrayList<>();
final ServiceLoader<OpensearchClientConfigurator> loaders = ServiceLoader
.load(OpensearchClientConfigurator.class, ClientsConfiguratorProvider.class.getClassLoader());

final Iterator<OpensearchClientConfigurator> iterator = loaders.iterator();
while (iterator.hasNext()) {
final OpensearchClientConfigurator configurator = iterator.next();
for (OpensearchClientConfigurator configurator : loaders) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be replaced with

return ServiceLoader
                .load(OpensearchClientConfigurator.class, ClientsConfiguratorProvider.class.getClassLoader()).stream().collect(Collectors.toList());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants