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/07/07 13:03:19 UTC
[GitHub] [kafka] lct45 opened a new pull request #10988: [KAFKA-9559] add docs for changing default serde to null
lct45 opened a new pull request #10988:
URL: https://github.com/apache/kafka/pull/10988
https://github.com/apache/kafka/pull/10813 changed the default serde from ByteArraySerde as discussed in [KIP-741](https://cwiki.apache.org/confluence/display/KAFKA/KIP-741%3A+Change+default+serde+to+be+null). This adds proper documentation so users know to set a serde through the configs or explicitly pass one in
### 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10988: [KAFKA-9559] add docs for changing default serde to null
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10988:
URL: https://github.com/apache/kafka/pull/10988#discussion_r666428748
##########
File path: docs/streams/developer-guide/datatypes.html
##########
@@ -34,7 +34,7 @@
<div class="section" id="data-types-and-serialization">
<span id="streams-developer-guide-serdes"></span><h1>Data Types and Serialization<a class="headerlink" href="#data-types-and-serialization" title="Permalink to this headline"></a></h1>
<p>Every Kafka Streams application must provide SerDes (Serializer/Deserializer) for the data types of record keys and record values (e.g. <code class="docutils literal"><span class="pre">java.lang.String</span></code>) to materialize the data when necessary. Operations that require such SerDes information include: <code class="docutils literal"><span class="pre">stream()</span></code>, <code class="docutils literal"><span class="pre">table()</span></code>, <code class="docutils literal"><span class="pre">to()</span></code>, <code class="docutils literal"><span class="pre">repartition()</span></code>, <code class="docutils literal"><span class="pre">groupByKey()</span></code>, <code class="docutils literal"><span class="pre">groupBy()</span></code>.</p>
- <p>You can provide SerDes by using either of these methods:</p>
+ <p>You can provide SerDes by using either of these methods, but you must use at least one of these methods:</p>
<ul class="simple">
<li>By setting default SerDes in the <code class="docutils literal"><span class="pre">java.util.Properties</span></code> config instance.</li>
<li>By specifying explicit SerDes when calling the appropriate API methods, thus overriding the defaults.</li>
Review comment:
Hm, dare I ask how many pages do this? Actually you should be able to do it using `sed` to programmatically go through each of the pages (if you've never used `sed` before it's a good thing to learn, you should be able to figure out how it works from googling for find&replace examples)
That said, let's leave it out of this PR. Can you just file a ticket to do this in a followup PR? Then I think this one can be merged
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] lct45 commented on pull request #10988: KAFKA-9559: add docs for changing default serde to null
Posted by GitBox <gi...@apache.org>.
lct45 commented on pull request #10988:
URL: https://github.com/apache/kafka/pull/10988#issuecomment-876679764
Thanks @ableegoldman ! created the follow-up here: https://issues.apache.org/jira/browse/KAFKA-13052
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] lct45 commented on a change in pull request #10988: [KAFKA-9559] add docs for changing default serde to null
Posted by GitBox <gi...@apache.org>.
lct45 commented on a change in pull request #10988:
URL: https://github.com/apache/kafka/pull/10988#discussion_r665531990
##########
File path: docs/streams/developer-guide/config-streams.html
##########
@@ -211,8 +211,9 @@ <h4><a class="toc-backref" href="#id5">bootstrap.servers</a><a class="headerlink
</tr>
<tr class="row-even"><td>default.key.serde</td>
<td>Medium</td>
- <td colspan="2">Default serializer/deserializer class for record keys, implements the <code class="docutils literal"><span class="pre">Serde</span></code> interface (see also default.value.serde).</td>
- <td><code class="docutils literal"><span class="pre">Serdes.ByteArray().getClass().getName()</span></code></td>
+ <td colspan="2">Default serializer/deserializer class for record keys, implements the <code class="docutils literal"><span class="pre">Serde</span></code> interface. Must be
Review comment:
Technically no (: it would just be null. Anything you pass in would implement the interface
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #10988: KAFKA-9559: add docs for changing default serde to null
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10988:
URL: https://github.com/apache/kafka/pull/10988#issuecomment-876657026
Merged to trunk and cherrypicked to 3.0
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] lct45 commented on a change in pull request #10988: [KAFKA-9559] add docs for changing default serde to null
Posted by GitBox <gi...@apache.org>.
lct45 commented on a change in pull request #10988:
URL: https://github.com/apache/kafka/pull/10988#discussion_r666177495
##########
File path: docs/streams/developer-guide/datatypes.html
##########
@@ -34,7 +34,7 @@
<div class="section" id="data-types-and-serialization">
<span id="streams-developer-guide-serdes"></span><h1>Data Types and Serialization<a class="headerlink" href="#data-types-and-serialization" title="Permalink to this headline"></a></h1>
<p>Every Kafka Streams application must provide SerDes (Serializer/Deserializer) for the data types of record keys and record values (e.g. <code class="docutils literal"><span class="pre">java.lang.String</span></code>) to materialize the data when necessary. Operations that require such SerDes information include: <code class="docutils literal"><span class="pre">stream()</span></code>, <code class="docutils literal"><span class="pre">table()</span></code>, <code class="docutils literal"><span class="pre">to()</span></code>, <code class="docutils literal"><span class="pre">repartition()</span></code>, <code class="docutils literal"><span class="pre">groupByKey()</span></code>, <code class="docutils literal"><span class="pre">groupBy()</span></code>.</p>
- <p>You can provide SerDes by using either of these methods:</p>
+ <p>You can provide SerDes by using either of these methods, but you must use at least one of these methods:</p>
<ul class="simple">
<li>By setting default SerDes in the <code class="docutils literal"><span class="pre">java.util.Properties</span></code> config instance.</li>
<li>By specifying explicit SerDes when calling the appropriate API methods, thus overriding the defaults.</li>
Review comment:
Sure, just did it for the `datatypes` page. Do you think we should do it for all uses in `dsl-api`?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10988: [KAFKA-9559] add docs for changing default serde to null
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10988:
URL: https://github.com/apache/kafka/pull/10988#discussion_r665708080
##########
File path: docs/streams/developer-guide/config-streams.html
##########
@@ -549,7 +551,7 @@ <h4><a class="toc-backref" href="#id27">acceptable.recovery.lag</a><a class="hea
<div class="section" id="default-key-serde">
<h4><a class="toc-backref" href="#id8">default.key.serde</a><a class="headerlink" href="#default-key-serde" title="Permalink to this headline"></a></h4>
<blockquote>
- <div><p>The default Serializer/Deserializer class for record keys. Serialization and deserialization in Kafka Streams happens
+ <div><p>The default Serializer/Deserializer class for record keys, null until set by user. Serialization and deserialization in Kafka Streams happens
Review comment:
nit:
```suggestion
<div><p>The default Serializer/Deserializer class for record keys, null unless set by user. Serialization and deserialization in Kafka Streams happens
```
##########
File path: docs/streams/developer-guide/datatypes.html
##########
@@ -34,7 +34,7 @@
<div class="section" id="data-types-and-serialization">
<span id="streams-developer-guide-serdes"></span><h1>Data Types and Serialization<a class="headerlink" href="#data-types-and-serialization" title="Permalink to this headline"></a></h1>
<p>Every Kafka Streams application must provide SerDes (Serializer/Deserializer) for the data types of record keys and record values (e.g. <code class="docutils literal"><span class="pre">java.lang.String</span></code>) to materialize the data when necessary. Operations that require such SerDes information include: <code class="docutils literal"><span class="pre">stream()</span></code>, <code class="docutils literal"><span class="pre">table()</span></code>, <code class="docutils literal"><span class="pre">to()</span></code>, <code class="docutils literal"><span class="pre">repartition()</span></code>, <code class="docutils literal"><span class="pre">groupByKey()</span></code>, <code class="docutils literal"><span class="pre">groupBy()</span></code>.</p>
- <p>You can provide SerDes by using either of these methods:</p>
+ <p>You can provide SerDes by using either of these methods, but you must use at least one of these methods:</p>
Review comment:
nit:
```suggestion
<p>You can provide Serdes by using either of these methods, but you must use at least one:</p>
```
##########
File path: docs/streams/developer-guide/config-streams.html
##########
@@ -562,7 +564,7 @@ <h4><a class="toc-backref" href="#id8">default.key.serde</a><a class="headerlink
<div class="section" id="default-value-serde">
<h4><a class="toc-backref" href="#id9">default.value.serde</a><a class="headerlink" href="#default-value-serde" title="Permalink to this headline"></a></h4>
<blockquote>
- <div><p>The default Serializer/Deserializer class for record values. Serialization and deserialization in Kafka Streams
+ <div><p>The default Serializer/Deserializer class for record values, null until set by user. Serialization and deserialization in Kafka Streams
Review comment:
```suggestion
<div><p>The default Serializer/Deserializer class for record values, null unless set by user. Serialization and deserialization in Kafka Streams
```
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -232,6 +232,11 @@ <h3><a id="streams_api_changes_280" href="#streams_api_changes_280">Streams API
Kafka Streams throws a <code>TimeoutException</code>
(cf. <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-572%3A+Improve+timeouts+and+retries+in+Kafka+Streams">KIP-572</a>).
</p>
+ <p>
+ We changed <code>default.key.serde</code> and <code>default.value.serde</code> to be <code>null</code> instead of <code>ByteArraySerde</code>.
Review comment:
```suggestion
We changed the default value of <code>default.key.serde</code> and <code>default.value.serde</code> to be <code>null</code> instead of <code>ByteArraySerde</code>.
```
##########
File path: docs/streams/developer-guide/datatypes.html
##########
@@ -54,7 +54,9 @@
</ul>
<div class="section" id="configuring-serdes">
<h2>Configuring SerDes<a class="headerlink" href="#configuring-serdes" title="Permalink to this headline"></a></h2>
- <p>SerDes specified in the Streams configuration are used as the default in your Kafka Streams application.</p>
+ <p>SerDes specified in the Streams configuration are used as the default in your Kafka Streams application.
+ Because this config's default is null, you must either set a default SerDe by using this
Review comment:
nit: alignment looks a bit off (not sure if it's supposed to be though?)
##########
File path: docs/streams/developer-guide/datatypes.html
##########
@@ -34,7 +34,7 @@
<div class="section" id="data-types-and-serialization">
<span id="streams-developer-guide-serdes"></span><h1>Data Types and Serialization<a class="headerlink" href="#data-types-and-serialization" title="Permalink to this headline"></a></h1>
<p>Every Kafka Streams application must provide SerDes (Serializer/Deserializer) for the data types of record keys and record values (e.g. <code class="docutils literal"><span class="pre">java.lang.String</span></code>) to materialize the data when necessary. Operations that require such SerDes information include: <code class="docutils literal"><span class="pre">stream()</span></code>, <code class="docutils literal"><span class="pre">table()</span></code>, <code class="docutils literal"><span class="pre">to()</span></code>, <code class="docutils literal"><span class="pre">repartition()</span></code>, <code class="docutils literal"><span class="pre">groupByKey()</span></code>, <code class="docutils literal"><span class="pre">groupBy()</span></code>.</p>
- <p>You can provide SerDes by using either of these methods:</p>
+ <p>You can provide SerDes by using either of these methods, but you must use at least one of these methods:</p>
<ul class="simple">
<li>By setting default SerDes in the <code class="docutils literal"><span class="pre">java.util.Properties</span></code> config instance.</li>
<li>By specifying explicit SerDes when calling the appropriate API methods, thus overriding the defaults.</li>
Review comment:
Think you can do a quick find&replace to standardize all of these `SerDes` to just `Serdes`, since that's how we now spell it everywhere else?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] lct45 commented on pull request #10988: [KAFKA-9559] add docs for changing default serde to null
Posted by GitBox <gi...@apache.org>.
lct45 commented on pull request #10988:
URL: https://github.com/apache/kafka/pull/10988#issuecomment-875584527
cc @ableegoldman @mjsax @wcarlson5 for review
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] wcarlson5 commented on a change in pull request #10988: [KAFKA-9559] add docs for changing default serde to null
Posted by GitBox <gi...@apache.org>.
wcarlson5 commented on a change in pull request #10988:
URL: https://github.com/apache/kafka/pull/10988#discussion_r665519455
##########
File path: docs/streams/developer-guide/config-streams.html
##########
@@ -211,8 +211,9 @@ <h4><a class="toc-backref" href="#id5">bootstrap.servers</a><a class="headerlink
</tr>
<tr class="row-even"><td>default.key.serde</td>
<td>Medium</td>
- <td colspan="2">Default serializer/deserializer class for record keys, implements the <code class="docutils literal"><span class="pre">Serde</span></code> interface (see also default.value.serde).</td>
- <td><code class="docutils literal"><span class="pre">Serdes.ByteArray().getClass().getName()</span></code></td>
+ <td colspan="2">Default serializer/deserializer class for record keys, implements the <code class="docutils literal"><span class="pre">Serde</span></code> interface. Must be
Review comment:
Does the default null still implements the interface?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman merged pull request #10988: KAFKA-9559: add docs for changing default serde to null
Posted by GitBox <gi...@apache.org>.
ableegoldman merged pull request #10988:
URL: https://github.com/apache/kafka/pull/10988
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org