Add topic client#58
Conversation
torbsto
left a comment
torbsto
left a comment
There was a problem hiding this comment.
Mostly minor things.
Besides this, there is one thing I would suggest. You always throw RuntimeException on encountering any problem. I think there are plenty of situations where you can recover from the error. I see that in most situations the application should stop. Therefore, checked exceptions maybe aren't ideal either. However, I suggest at least using custom exceptions so that you can catch them easily.
| private final @NonNull Duration timeout; | ||
|
|
||
| /** | ||
| * Create a new {@code TopicClient} using the specified configuration. |
There was a problem hiding this comment.
| * Create a new {@code TopicClient} using the specified configuration. | |
| * Creates a new {@code TopicClient} using the specified configuration. |
| public void createIfNotExists(final String topicName, final TopicSettings settings, | ||
| final Map<String, String> config) { | ||
| if (this.exists(topicName)) { | ||
| log.debug("Topic {} already exists, no need to create.", topicName); |
There was a problem hiding this comment.
Seems more like an info level log statement to me?
| throw new RuntimeException("Failed to check if Kafka topic " + topicName + " exists", e); | ||
| } | ||
| } catch (final InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| throw new RuntimeException("Failed to check if Kafka topic " + topicName + " exists", e); | ||
| } catch (final TimeoutException e) { | ||
| throw new RuntimeException("Failed to check if Kafka topic " + topicName + " exists", e); | ||
| } |
There was a problem hiding this comment.
couldn't you throw the exception just once after the try-catch block? All valid responses are in early returns anyway
There was a problem hiding this comment.
We have to handle the exceptions differently. InterruptedExceptions require Thread.currentThread().interrupt();
| .build(); | ||
| this.kafkaCluster = provisionWith(clusterConfig); | ||
| this.kafkaCluster.start(); | ||
| Thread.sleep(TimeUnit.SECONDS.toMillis(TIMEOUT_SECONDS)); |
There was a problem hiding this comment.
It should work without the timeout
There was a problem hiding this comment.
I'm unsure about this because I had to restart the build once...
| } | ||
|
|
||
| /** | ||
| * Check whether a Kafka topic exists. |
There was a problem hiding this comment.
| * Check whether a Kafka topic exists. | |
| * Checks whether a Kafka topic exists. |
| } | ||
|
|
||
| /** | ||
| * Describe the current configuration of a Kafka topic. |
There was a problem hiding this comment.
| * Describe the current configuration of a Kafka topic. | |
| * Describes the current configuration of a Kafka topic. |
2 participants
No description provided.