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/23 15:41:53 UTC
[GitHub] [kafka] chia7712 opened a new pull request #10193: MINOR: correct the error message of validating uint32
chia7712 opened a new pull request #10193:
URL: https://github.com/apache/kafka/pull/10193
as title
### 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] tombentley commented on a change in pull request #10193: MINOR: correct the error message of validating uint32
Posted by GitBox <gi...@apache.org>.
tombentley commented on a change in pull request #10193:
URL: https://github.com/apache/kafka/pull/10193#discussion_r584648224
##########
File path: clients/src/main/java/org/apache/kafka/common/protocol/types/Type.java
##########
@@ -320,7 +320,7 @@ public Long validate(Object item) {
if (item instanceof Long)
return (Long) item;
else
- throw new SchemaException(item + " is not a Long.");
+ throw new SchemaException(item + " is not an unsigned integer.");
Review comment:
Though now I look at the message for `UINT16` I see it would be consistent with that. Still I think because there are two types involved here, the Java type and the network type, including both is clearest.
----------------------------------------------------------------
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] ijuma commented on a change in pull request #10193: MINOR: correct the error message of validating uint32
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10193:
URL: https://github.com/apache/kafka/pull/10193#discussion_r585897929
##########
File path: clients/src/main/java/org/apache/kafka/common/protocol/types/Type.java
##########
@@ -320,7 +320,7 @@ public Long validate(Object item) {
if (item instanceof Long)
return (Long) item;
else
- throw new SchemaException(item + " is not a Long.");
+ throw new SchemaException(item + " is not an a Long (encoding an unsigned integer).");
Review comment:
There is a typo here, "an 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] chia7712 commented on a change in pull request #10193: MINOR: correct the error message of validating uint32
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10193:
URL: https://github.com/apache/kafka/pull/10193#discussion_r586082468
##########
File path: clients/src/main/java/org/apache/kafka/common/protocol/types/Type.java
##########
@@ -320,7 +320,7 @@ public Long validate(Object item) {
if (item instanceof Long)
return (Long) item;
else
- throw new SchemaException(item + " is not a Long.");
+ throw new SchemaException(item + " is not an a Long (encoding an unsigned integer).");
Review comment:
> There is a typo here, "an a".
My bad :(
> Also, I think it reads a bit weird. Not clear that "encoding an unsigned integer" in brackets means when reading the message.
I will update it in #10248 @ijuma thanks for your reviews
----------------------------------------------------------------
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 #10193: MINOR: correct the error message of validating uint32
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10193:
URL: https://github.com/apache/kafka/pull/10193#discussion_r585237560
##########
File path: clients/src/main/java/org/apache/kafka/common/protocol/types/Type.java
##########
@@ -320,7 +320,7 @@ public Long validate(Object item) {
if (item instanceof Long)
return (Long) item;
else
- throw new SchemaException(item + " is not a Long.");
+ throw new SchemaException(item + " is not an unsigned integer.");
Review comment:
make sense. will copy that.
----------------------------------------------------------------
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] ijuma commented on a change in pull request #10193: MINOR: correct the error message of validating uint32
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10193:
URL: https://github.com/apache/kafka/pull/10193#discussion_r585899128
##########
File path: clients/src/main/java/org/apache/kafka/common/protocol/types/Type.java
##########
@@ -320,7 +320,7 @@ public Long validate(Object item) {
if (item instanceof Long)
return (Long) item;
else
- throw new SchemaException(item + " is not a Long.");
+ throw new SchemaException(item + " is not an a Long (encoding an unsigned integer).");
Review comment:
Also, I think it reads a bit weird. Not clear that "encoding an unsigned integer" in brackets means when reading the message.
----------------------------------------------------------------
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 #10193: MINOR: correct the error message of validating uint32
Posted by GitBox <gi...@apache.org>.
chia7712 merged pull request #10193:
URL: https://github.com/apache/kafka/pull/10193
----------------------------------------------------------------
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] tombentley commented on a change in pull request #10193: MINOR: correct the error message of validating uint32
Posted by GitBox <gi...@apache.org>.
tombentley commented on a change in pull request #10193:
URL: https://github.com/apache/kafka/pull/10193#discussion_r584646665
##########
File path: clients/src/main/java/org/apache/kafka/common/protocol/types/Type.java
##########
@@ -320,7 +320,7 @@ public Long validate(Object item) {
if (item instanceof Long)
return (Long) item;
else
- throw new SchemaException(item + " is not a Long.");
+ throw new SchemaException(item + " is not an unsigned integer.");
Review comment:
This would be inconsistent with the other messages, which refer to Java types. It's a condition on the Java type, so including it in the message is reasonable imho.
```suggestion
throw new SchemaException(item + " is not a Long (encoding an unsigned integer).");
```
----------------------------------------------------------------
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