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/02/19 14:10:03 UTC
[GitHub] [kafka] dengziming opened a new pull request #10162: DOCS: Update protocol doc for missing data type
dengziming opened a new pull request #10162:
URL: https://github.com/apache/kafka/pull/10162
*More detailed description of your change*
Add missing records(KAFKA-9629) uint16(KAFKA-12180) docs.
*Summary of testing strategy (including rationale)*
No
### 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] chia7712 commented on a change in pull request #10162: DOCS: Update protocol doc for missing data type
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10162:
URL: https://github.com/apache/kafka/pull/10162#discussion_r582178585
##########
File path: clients/src/main/resources/common/message/README.md
##########
@@ -75,16 +75,24 @@ There are several primitive field types available.
* "int16": a 16-bit integer.
+* "uint16": a 16-bit unsigned integer.
+
* "int32": a 32-bit integer.
+* "uint32": a 32-bit unsigned integer.
+
* "int64": a 64-bit integer.
* "float64": is a double-precision floating point number (IEEE 754).
* "string": a UTF-8 string.
+* "uuid": a type 4 immutable universally unique identifier.
+
* "bytes": binary data.
+* "records": memory record set or file record set.
Review comment:
the supported records type in KRPC is only `MemoryRecords`, right?
----------------------------------------------------------------
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] chia7712 commented on a change in pull request #10162: DOCS: Update protocol doc for missing data type
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10162:
URL: https://github.com/apache/kafka/pull/10162#discussion_r582529627
##########
File path: clients/src/main/resources/common/message/README.md
##########
@@ -75,16 +75,24 @@ There are several primitive field types available.
* "int16": a 16-bit integer.
+* "uint16": a 16-bit unsigned integer.
+
* "int32": a 32-bit integer.
+* "uint32": a 32-bit unsigned integer.
+
* "int64": a 64-bit integer.
* "float64": is a double-precision floating point number (IEEE 754).
* "string": a UTF-8 string.
+* "uuid": a type 4 immutable universally unique identifier.
+
* "bytes": binary data.
+* "records": memory record set or file record set.
Review comment:
There are many sub classes for this type as the java type used by KRPC is `BaseRecords`. We do make protocol data carry different sub class in processing data (before serialization). Hence, it seems to me listing sub classes in this document is a bit weird.
----------------------------------------------------------------
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] dengziming commented on a change in pull request #10162: DOCS: Update protocol doc for missing data type
Posted by GitBox <gi...@apache.org>.
dengziming commented on a change in pull request #10162:
URL: https://github.com/apache/kafka/pull/10162#discussion_r581577823
##########
File path: clients/src/main/resources/common/message/README.md
##########
@@ -75,6 +75,8 @@ There are several primitive field types available.
* "int16": a 16-bit integer.
+* "uint16": a 16-bit unsigned integer.
Review comment:
Added uint32 and uuid.
----------------------------------------------------------------
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] chia7712 commented on a change in pull request #10162: DOCS: Update protocol doc for missing data type
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10162:
URL: https://github.com/apache/kafka/pull/10162#discussion_r581129848
##########
File path: clients/src/main/resources/common/message/README.md
##########
@@ -75,6 +75,8 @@ There are several primitive field types available.
* "int16": a 16-bit integer.
+* "uint16": a 16-bit unsigned integer.
Review comment:
Could you add document for "unit32"?
----------------------------------------------------------------
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] dengziming commented on a change in pull request #10162: DOCS: Update protocol doc for missing data type
Posted by GitBox <gi...@apache.org>.
dengziming commented on a change in pull request #10162:
URL: https://github.com/apache/kafka/pull/10162#discussion_r579639403
##########
File path: clients/src/main/resources/common/message/README.md
##########
@@ -85,6 +87,8 @@ There are several primitive field types available.
* "bytes": binary data.
+* "records": record set used in fetch api and fetch snapshot api
Review comment:
Thank you, I think it's better to remove usage cases or we need to update docs every time we use it, I reworded this doc, PTAL.
----------------------------------------------------------------
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] chia7712 commented on a change in pull request #10162: DOCS: Update protocol doc for missing data type
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10162:
URL: https://github.com/apache/kafka/pull/10162#discussion_r582587709
##########
File path: clients/src/main/resources/common/message/README.md
##########
@@ -75,16 +75,24 @@ There are several primitive field types available.
* "int16": a 16-bit integer.
+* "uint16": a 16-bit unsigned integer.
+
* "int32": a 32-bit integer.
+* "uint32": a 32-bit unsigned integer.
+
* "int64": a 64-bit integer.
* "float64": is a double-precision floating point number (IEEE 754).
* "string": a UTF-8 string.
+* "uuid": a type 4 immutable universally unique identifier.
+
* "bytes": binary data.
+* "records": memory record set or file record set.
Review comment:
That LGTM :)
----------------------------------------------------------------
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] chia7712 merged pull request #10162: DOCS: Update protocol doc for missing data type
Posted by GitBox <gi...@apache.org>.
chia7712 merged pull request #10162:
URL: https://github.com/apache/kafka/pull/10162
----------------------------------------------------------------
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] dengziming commented on a change in pull request #10162: DOCS: Update protocol doc for missing data type
Posted by GitBox <gi...@apache.org>.
dengziming commented on a change in pull request #10162:
URL: https://github.com/apache/kafka/pull/10162#discussion_r582432795
##########
File path: clients/src/main/resources/common/message/README.md
##########
@@ -75,16 +75,24 @@ There are several primitive field types available.
* "int16": a 16-bit integer.
+* "uint16": a 16-bit unsigned integer.
+
* "int32": a 32-bit integer.
+* "uint32": a 32-bit unsigned integer.
+
* "int64": a 64-bit integer.
* "float64": is a double-precision floating point number (IEEE 754).
* "string": a UTF-8 string.
+* "uuid": a type 4 immutable universally unique identifier.
+
* "bytes": binary data.
+* "records": memory record set or file record set.
Review comment:
Only `MemoryRecords` are supported when receiving records, but we can also set `FileRecords` when writing records. foe example `KafkaRaftClient.tryCompleteFetchRequest()` will call `partitionData.setRecordSet(log.read(xxx))`
----------------------------------------------------------------
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] dengziming commented on a change in pull request #10162: DOCS: Update protocol doc for missing data type
Posted by GitBox <gi...@apache.org>.
dengziming commented on a change in pull request #10162:
URL: https://github.com/apache/kafka/pull/10162#discussion_r582564994
##########
File path: clients/src/main/resources/common/message/README.md
##########
@@ -75,16 +75,24 @@ There are several primitive field types available.
* "int16": a 16-bit integer.
+* "uint16": a 16-bit unsigned integer.
+
* "int32": a 32-bit integer.
+* "uint32": a 32-bit unsigned integer.
+
* "int64": a 64-bit integer.
* "float64": is a double-precision floating point number (IEEE 754).
* "string": a UTF-8 string.
+* "uuid": a type 4 immutable universally unique identifier.
+
* "bytes": binary data.
+* "records": memory record set or file record set.
Review comment:
I think we should avoid listing subclasses since we would have to update docs every time we add a new subclass, how about rewording this to "recordset such as memory recordset"?
----------------------------------------------------------------
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] dengziming commented on pull request #10162: DOCS: Update protocol doc for missing data type
Posted by GitBox <gi...@apache.org>.
dengziming commented on pull request #10162:
URL: https://github.com/apache/kafka/pull/10162#issuecomment-782101523
@mumrah @cmccabe , Hello, the 2 data types (records, uint16) are imported by you, PTAL.
----------------------------------------------------------------
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] chia7712 commented on a change in pull request #10162: DOCS: Update protocol doc for missing data type
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10162:
URL: https://github.com/apache/kafka/pull/10162#discussion_r579615450
##########
File path: clients/src/main/resources/common/message/README.md
##########
@@ -85,6 +87,8 @@ There are several primitive field types available.
* "bytes": binary data.
+* "records": record set used in fetch api and fetch snapshot api
Review comment:
`records` is used by produce data also.
----------------------------------------------------------------
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