mjsax commented on code in PR #19913:
URL: https://github.com/apache/kafka/pull/19913#discussion_r2162801345
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/InternalTopicManager.java:
##########
@@ -90,6 +92,8 @@ public InternalTopicManager(final Time time,
final StreamsConfig streamsConfig) {
this.time = time;
this.adminClient = adminClient;
+ this.isManualInternalTopicConfig =
streamsConfig.getString(StreamsConfig.INTERNAL_TOPIC_SETUP_CONFIG)
+
.equals(StreamsConfig.INTERNAL_TOPIC_SETUP_MANUAL);
Review Comment:
> So the way you wrote this does functionally the same thing, but I'm
introducing some risk that can be avoided by just changing the order?
Correct.
> But it's possible that if someone isn't using this feature, or it's not
defaulted etc. that a crash is caused without good reason?
Yes. For this particular case, it does not matter too much, but in general,
there is always a change that `<variable> == null` so you it's just best
practice to not use `<variable>.equals(...)` w/o a `null` check for
`<variable>` (by flipping to `<constant>.equals(...)` we don't need the `null`
check and are safe.)
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
