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/10/08 06:16:37 UTC
[GitHub] [kafka] chia7712 opened a new pull request #9391: MINOR: fix potential NPT in PartitionData.equals
chia7712 opened a new pull request #9391:
URL: https://github.com/apache/kafka/pull/9391
the field ```metadata``` is nullable (see https://github.com/apache/kafka/blob/trunk/clients/src/main/resources/common/message/OffsetFetchResponse.json#L50)
### 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 #9391: MINOR: fix potential NPT in PartitionData.equals
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9391:
URL: https://github.com/apache/kafka/pull/9391#discussion_r501507673
##########
File path: clients/src/test/java/org/apache/kafka/common/requests/OffsetFetchResponseTest.java
##########
@@ -159,14 +159,14 @@ public void testShouldThrottle() {
@Test
public void testNullableMetadata() {
+ PartitionData pd = new PartitionData(
+ offset,
+ leaderEpochOne,
+ null,
+ Errors.UNKNOWN_TOPIC_OR_PARTITION);
+ assertEquals(pd, pd);
Review comment:
thanks for this quickly and meaningful review and I 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] dajac commented on a change in pull request #9391: MINOR: fix potential NPT in PartitionData.equals
Posted by GitBox <gi...@apache.org>.
dajac commented on a change in pull request #9391:
URL: https://github.com/apache/kafka/pull/9391#discussion_r501505455
##########
File path: clients/src/test/java/org/apache/kafka/common/requests/OffsetFetchResponseTest.java
##########
@@ -159,14 +159,14 @@ public void testShouldThrottle() {
@Test
public void testNullableMetadata() {
+ PartitionData pd = new PartitionData(
+ offset,
+ leaderEpochOne,
+ null,
+ Errors.UNKNOWN_TOPIC_OR_PARTITION);
+ assertEquals(pd, pd);
Review comment:
I believe that one could be easily tempted to remove such assertion without knowing the context. I would add a comment to explain it or define a dedicate test case for the bug. I am fine with either ways.
----------------------------------------------------------------
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 #9391: MINOR: fix potential NPT in PartitionData.equals
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9391:
URL: https://github.com/apache/kafka/pull/9391#discussion_r501507673
##########
File path: clients/src/test/java/org/apache/kafka/common/requests/OffsetFetchResponseTest.java
##########
@@ -159,14 +159,14 @@ public void testShouldThrottle() {
@Test
public void testNullableMetadata() {
+ PartitionData pd = new PartitionData(
+ offset,
+ leaderEpochOne,
+ null,
+ Errors.UNKNOWN_TOPIC_OR_PARTITION);
+ assertEquals(pd, pd);
Review comment:
thanks for this quickly and meaningful review and I 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] chia7712 commented on pull request #9391: MINOR: fix potential NPT in PartitionData.equals
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9391:
URL: https://github.com/apache/kafka/pull/9391#issuecomment-711167818
```
Build / JDK 8 / org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[true]
```
unrelated error
----------------------------------------------------------------
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 pull request #9391: MINOR: fix potential NPT in PartitionData.equals
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9391:
URL: https://github.com/apache/kafka/pull/9391#issuecomment-711167896
@dajac Thanks for reviews. Merge it to trunk
----------------------------------------------------------------
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 #9391: MINOR: fix potential NPE in PartitionData.equals
Posted by GitBox <gi...@apache.org>.
chia7712 merged pull request #9391:
URL: https://github.com/apache/kafka/pull/9391
----------------------------------------------------------------
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] dajac commented on a change in pull request #9391: MINOR: fix potential NPT in PartitionData.equals
Posted by GitBox <gi...@apache.org>.
dajac commented on a change in pull request #9391:
URL: https://github.com/apache/kafka/pull/9391#discussion_r501505455
##########
File path: clients/src/test/java/org/apache/kafka/common/requests/OffsetFetchResponseTest.java
##########
@@ -159,14 +159,14 @@ public void testShouldThrottle() {
@Test
public void testNullableMetadata() {
+ PartitionData pd = new PartitionData(
+ offset,
+ leaderEpochOne,
+ null,
+ Errors.UNKNOWN_TOPIC_OR_PARTITION);
+ assertEquals(pd, pd);
Review comment:
I believe that one could be easily tempted to remove such assertion without knowing the context. I would add a comment to explain it or define a dedicate test case for the bug. I am fine with either ways.
----------------------------------------------------------------
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