You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/06/08 19:02:12 UTC
[GitHub] [kafka] mjsax opened a new pull request #10846: KAFKA-12914: StreamSourceNode should return `null` topic name for pattern subscription
mjsax opened a new pull request #10846:
URL: https://github.com/apache/kafka/pull/10846
Call for review @cadonna
--
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
[GitHub] [kafka] mjsax commented on pull request #10846: KAFKA-12914: StreamSourceNode should return `null` topic name for pattern subscription
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #10846:
URL: https://github.com/apache/kafka/pull/10846#issuecomment-858160278
What do you propose @cadonna ? Also happy to do a follow up PR (but not sure if necessary)? It's all just internal code so we can change at will. Happy to merge as-is, or change to `Optional` in this or a follow up PR...
--
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
[GitHub] [kafka] mjsax commented on pull request #10846: KAFKA-12914: StreamSourceNode should return `null` topic name for pattern subscription
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #10846:
URL: https://github.com/apache/kafka/pull/10846#issuecomment-861288799
Merged to `trunk` and cherry-picked to `2.8` branch.
--
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
[GitHub] [kafka] cadonna commented on pull request #10846: KAFKA-12914: StreamSourceNode should return `null` topic name for pattern subscription
Posted by GitBox <gi...@apache.org>.
cadonna commented on pull request #10846:
URL: https://github.com/apache/kafka/pull/10846#issuecomment-857476064
`Optional` sounds good but it would substantially increase the size of this fix which is actually not required.
On the other hand, I am not sure if we do not have other related NPE lurking in the code that should be found by changing to `Optional`. For example, I could not find a null check for https://github.com/apache/kafka/blob/45d7440c1577b838d4584d3860e5c4d691446f3f/streams/src/main/java/org/apache/kafka/streams/kstream/internals/graph/TableSourceNode.java#L86
--
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
[GitHub] [kafka] mjsax commented on pull request #10846: KAFKA-12914: StreamSourceNode should return `null` topic name for pattern subscription
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #10846:
URL: https://github.com/apache/kafka/pull/10846#issuecomment-857329691
> Just a thought that do you think it's better to return empty set when null?
For `SourceGraphNode` we either set `topicName` or `pattern` `null`, and we also return `null` for the pattern case if `topicName` is used and `pattern is `null`. So I thought it might be better aligned to just return `null` if `topicName` is `null`, too. But I don't have a strong opinion.
As an afterthought, it might even be better to change both return types to `Optional` ?
--
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
[GitHub] [kafka] showuon commented on pull request #10846: KAFKA-12914: StreamSourceNode should return `null` topic name for pattern subscription
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10846:
URL: https://github.com/apache/kafka/pull/10846#issuecomment-857330899
> As an afterthought, it might even be better to change both return types to `Optional` ?
Sounds good!
--
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
[GitHub] [kafka] mjsax commented on pull request #10846: KAFKA-12914: StreamSourceNode should return `null` topic name for pattern subscription
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #10846:
URL: https://github.com/apache/kafka/pull/10846#issuecomment-859236418
Update the PR to change the return type to Optional. If no objections are raised and Jenkins passed, I plan to merge this next week.
--
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
[GitHub] [kafka] mjsax merged pull request #10846: KAFKA-12914: StreamSourceNode should return `null` topic name for pattern subscription
Posted by GitBox <gi...@apache.org>.
mjsax merged pull request #10846:
URL: https://github.com/apache/kafka/pull/10846
--
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