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/07/01 07:58:52 UTC

[GitHub] [kafka] highluck opened a new pull request #8965: KAFKA-8147: Add changelog topic configuration to KTable suppress

highluck opened a new pull request #8965:
URL: https://github.com/apache/kafka/pull/8965


   KAFKA-8147: Add changelog topic configuration to KTable suppress
   
   ### 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] highluck commented on a change in pull request #8965: KAFKA-8147: Add changelog topic configuration to KTable suppress

Posted by GitBox <gi...@apache.org>.
highluck commented on a change in pull request #8965:
URL: https://github.com/apache/kafka/pull/8965#discussion_r533863847



##########
File path: docs/streams/developer-guide/dsl-api.html
##########
@@ -3393,6 +3393,10 @@ <h5><a class="toc-backref" href="#id34">KTable-KTable Foreign-Key
 				      but this simple example creates a buffer with no
 				      upper bound.
 				    </dd>
+                    <dt><code>withLoggingDisabled()</code></dt>
+                    <dd>
+                      This configures the suppression operator to disable logging for changelog entries.
+                    </dd>

Review comment:
       update code thanks!




----------------------------------------------------------------
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 #8965: KAFKA-8147: Add changelog topic configuration to KTable suppress

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


   Thanks a lot for the PR @highluck!


----------------------------------------------------------------
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] vvcephei commented on a change in pull request #8965: KAFKA-8147: Add changelog topic configuration to KTable suppress

Posted by GitBox <gi...@apache.org>.
vvcephei commented on a change in pull request #8965:
URL: https://github.com/apache/kafka/pull/8965#discussion_r533507421



##########
File path: docs/streams/developer-guide/dsl-api.html
##########
@@ -3393,6 +3393,10 @@ <h5><a class="toc-backref" href="#id34">KTable-KTable Foreign-Key
 				      but this simple example creates a buffer with no
 				      upper bound.
 				    </dd>
+                    <dt><code>withLoggingDisabled()</code></dt>
+                    <dd>
+                      This configures the suppression operator to disable logging for changelog entries.
+                    </dd>

Review comment:
       Thanks @mjsax , I agree; this is an advanced configuration that shouldn't be used most of the time, so I wouldn't put it in the example.




----------------------------------------------------------------
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 a change in pull request #8965: KAFKA-8147: Add changelog topic configuration to KTable suppress

Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #8965:
URL: https://github.com/apache/kafka/pull/8965#discussion_r525425175



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -120,6 +120,11 @@ <h3><a id="streams_api_changes_260" href="#streams_api_changes_260">Streams API
         We added a <code>--force</code> option in StreamsResetter to force remove left-over members on broker side when long session time out was configured
         as per <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-571%3A+Add+option+to+force+remove+members+in+StreamsResetter">KIP-571</a>.
     </p>
+    <p>
+        We added a <code>Suppressed.withLoggingDisabled()</code> and <code>Suppressed.withLoggingEnabled(config)</code>
+        method to disables (no s) and allows for configuration of the changelog topic

Review comment:
       `method[s]` (plural)
   
   `to disables (no s)...` -> `to allow disabling or configuring of the changelog topic`

##########
File path: docs/streams/upgrade-guide.html
##########
@@ -120,6 +120,11 @@ <h3><a id="streams_api_changes_260" href="#streams_api_changes_260">Streams API
         We added a <code>--force</code> option in StreamsResetter to force remove left-over members on broker side when long session time out was configured
         as per <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-571%3A+Add+option+to+force+remove+members+in+StreamsResetter">KIP-571</a>.
     </p>
+    <p>
+        We added a <code>Suppressed.withLoggingDisabled()</code> and <code>Suppressed.withLoggingEnabled(config)</code>

Review comment:
       just `We added <code>` (no `a`)




----------------------------------------------------------------
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] highluck commented on pull request #8965: KAFKA-8147: Add changelog topic configuration to KTable suppress

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


   @mjsax
   Should I do it like this way?
   thank you!


----------------------------------------------------------------
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 a change in pull request #8965: KAFKA-8147: Add changelog topic configuration to KTable suppress

Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #8965:
URL: https://github.com/apache/kafka/pull/8965#discussion_r451043527



##########
File path: docs/streams/upgrade-guide.html
##########
@@ -120,6 +120,11 @@ <h3><a id="streams_api_changes_260" href="#streams_api_changes_260">Streams API
         We added a <code>--force</code> option in StreamsResetter to force remove left-over members on broker side when long session time out was configured
         as per <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-571%3A+Add+option+to+force+remove+members+in+StreamsResetter">KIP-571</a>.
     </p>
+    <p>
+        We added a <code>Suppressed.withLoggingDisabled()</code> and <code>Suppressed.withLoggingEnabled(config)</code>
+        Suppressed API to disables logging and allows for configuration of the changelog topic

Review comment:
       `Suppressed API` -> `method`
   
   `to disable` (no `s`)
   `and allowing`

##########
File path: docs/streams/upgrade-guide.html
##########
@@ -120,6 +120,11 @@ <h3><a id="streams_api_changes_260" href="#streams_api_changes_260">Streams API
         We added a <code>--force</code> option in StreamsResetter to force remove left-over members on broker side when long session time out was configured
         as per <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-571%3A+Add+option+to+force+remove+members+in+StreamsResetter">KIP-571</a>.
     </p>
+    <p>
+        We added a <code>Suppressed.withLoggingDisabled()</code> and <code>Suppressed.withLoggingEnabled(config)</code>
+        Suppressed API to disables logging and allows for configuration of the changelog topic
+        as per <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-446%3A+Add+changelog+topic+configuration+to+KTable+suppress"></a>

Review comment:
       `<a href="...">KIP-446</a>.`
   
   There should be a link text :)
   
   Also add `.` at the end of the sentence.

##########
File path: docs/streams/developer-guide/dsl-api.html
##########
@@ -3393,6 +3393,10 @@ <h5><a class="toc-backref" href="#id34">KTable-KTable Foreign-Key
 				      but this simple example creates a buffer with no
 				      upper bound.
 				    </dd>
+                    <dt><code>withLoggingDisabled()</code></dt>
+                    <dd>
+                      This configures the suppression operator to disable logging for changelog entries.
+                    </dd>

Review comment:
       Not sure if we want to add this in this example? \cc @vvcephei WDYT?




----------------------------------------------------------------
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] highluck commented on pull request #8965: KAFKA-8147: Add changelog topic configuration to KTable suppress

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


   @mjsax 
   thank you for review!
   updated code!


----------------------------------------------------------------
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 #8965: KAFKA-8147: Add changelog topic configuration to KTable suppress

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


   Merged to `trunk` and cherry-picked to `2.7` and `2.6` branches.


----------------------------------------------------------------
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 #8965: KAFKA-8147: Add changelog topic configuration to KTable suppress

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


   


----------------------------------------------------------------
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