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 2020/06/07 06:37:47 UTC

[GitHub] [kafka] robin5540 opened a new pull request #8825: KAFKA-7833: Add name conflict check in StreamsGraphNode

robin5540 opened a new pull request #8825:
URL: https://github.com/apache/kafka/pull/8825


   *Summary of testing strategy (including rationale)
   * Added a test that attempts to call `.addStateStore` with the same `StoreBuilder`, which builds two stores of the same name.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


----------------------------------------------------------------
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 #8825: KAFKA-7833: Add StateStore name conflict check in InternalTopologyBuilder

Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8825:
URL: https://github.com/apache/kafka/pull/8825#issuecomment-642268513






----------------------------------------------------------------
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 #8825: KAFKA-7833: Add StateStore name conflict check in InternalTopologyBuilder

Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8825:
URL: https://github.com/apache/kafka/pull/8825#issuecomment-642287780


   Thanks for pointing it out. The info is not correct any more. I just updated the wiki.
   
   ASF infra team decided to protect Jenkins and now builds need to be triggered by committers.


----------------------------------------------------------------
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 #8825: KAFKA-7833: Add StateStore name conflict check in InternalStreamsBuilder

Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8825:
URL: https://github.com/apache/kafka/pull/8825#issuecomment-640280224


   Thanks for the PR. I looked into the code and the check to verify if names are overlapping is actually in `InternalTopologyBuilder` -- thus, the fix should there.
   
   In `InternalTopologyBuilderTest`, there is already `shouldNotAllowToAddStoresWithSameName` - - but it only covers the case for two "regular" store. The bug is really for adding a regular store and global store with the same name. Seems, we also lack a test for adding two global stores with the same names.
   
   Also note, that for adding a regular store first, and afterwards a global store, there seems to be not bug atm -- only if the global store is added first and the regular store second the issue is there. Thus, we should test both combinations.
   
   Does this make sense?


----------------------------------------------------------------
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] robin5540 commented on pull request #8825: KAFKA-7833: Add StateStore name conflict check in InternalStreamsBuilder

Posted by GitBox <gi...@apache.org>.
robin5540 commented on pull request #8825:
URL: https://github.com/apache/kafka/pull/8825#issuecomment-640293871


   @mjsax Makes sense. I'll push out the change tonight 👍 


----------------------------------------------------------------
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] robin5540 commented on pull request #8825: KAFKA-7833: Add StateStore name conflict check in InternalTopologyBuilder

Posted by GitBox <gi...@apache.org>.
robin5540 commented on pull request #8825:
URL: https://github.com/apache/kafka/pull/8825#issuecomment-642271699


   @mjsax I might have misread something somewhere. How do I trigger test explicitly? https://cwiki.apache.org/confluence/display/KAFKA/Contributing+Code+Changes seems to suggest that tests are automatically triggered on PRs.


----------------------------------------------------------------
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 #8825: KAFKA-7833: Add StateStore name conflict check in InternalTopologyBuilder

Posted by GitBox <gi...@apache.org>.
mjsax merged pull request #8825:
URL: https://github.com/apache/kafka/pull/8825


   


----------------------------------------------------------------
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 #8825: KAFKA-7833: Add StateStore name conflict check in InternalTopologyBuilder

Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8825:
URL: https://github.com/apache/kafka/pull/8825#issuecomment-642331899


   Thanks for the PR! Merged to `trunk` and cherry-picked to `2.6` 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