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/10/17 00:45:29 UTC
[GitHub] [kafka] ijuma opened a new pull request #11403: MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order
ijuma opened a new pull request #11403:
URL: https://github.com/apache/kafka/pull/11403
`TopicPartition` allows a null `topic` and there are cases where we have
a topic id, but no topic name. Even for `TopicIdPartition`, the non null
topic name check was only enforced in one constructor.
Also adjust the constructor order to move the nullable parameter to the
end, update tests and javadoc.
### 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] dajac commented on pull request #11403: MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order
Posted by GitBox <gi...@apache.org>.
dajac commented on pull request #11403:
URL: https://github.com/apache/kafka/pull/11403#issuecomment-964399588
Thanks @ijuma. Could you also cherry-pick to 3.1 branch?
--
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] ijuma commented on pull request #11403: MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #11403:
URL: https://github.com/apache/kafka/pull/11403#issuecomment-965182096
Merged to trunk and 3.1 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dajac commented on pull request #11403: MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order
Posted by GitBox <gi...@apache.org>.
dajac commented on pull request #11403:
URL: https://github.com/apache/kafka/pull/11403#issuecomment-964017256
@ijuma Could we merge this PR?
--
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] showuon commented on a change in pull request #11403: MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11403:
URL: https://github.com/apache/kafka/pull/11403#discussion_r738891900
##########
File path: clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java
##########
@@ -47,7 +58,7 @@ public Uuid topicId() {
}
/**
- * @return the topic name.
+ * @return the topic name or null.
Review comment:
You're right. "the topic name or null if it is unknown" is what I meant.
--
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] ijuma commented on a change in pull request #11403: MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #11403:
URL: https://github.com/apache/kafka/pull/11403#discussion_r738812162
##########
File path: clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java
##########
@@ -47,7 +58,7 @@ public Uuid topicId() {
}
/**
- * @return the topic name.
+ * @return the topic name or null.
Review comment:
Not sure if that adds any value. The topic id is always set, so how is saying "there's only topic id set" helping? Maybe "the topic name or null if it is unknown"?
--
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] ijuma commented on pull request #11403: MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #11403:
URL: https://github.com/apache/kafka/pull/11403#issuecomment-964337484
@dajac I'll merge after the build completes.
--
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] ijuma merged pull request #11403: MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order
Posted by GitBox <gi...@apache.org>.
ijuma merged pull request #11403:
URL: https://github.com/apache/kafka/pull/11403
--
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] showuon commented on a change in pull request #11403: MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11403:
URL: https://github.com/apache/kafka/pull/11403#discussion_r730753939
##########
File path: clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java
##########
@@ -47,7 +58,7 @@ public Uuid topicId() {
}
/**
- * @return the topic name.
+ * @return the topic name or null.
Review comment:
I think we should add more explanation for null return value:
ex: @return the topic name or null if there's only topic ID set.
--
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] ijuma commented on pull request #11403: MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #11403:
URL: https://github.com/apache/kafka/pull/11403#issuecomment-944435674
cc @jolshan
--
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