You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "m1a2st (via GitHub)" <gi...@apache.org> on 2026/02/16 13:07:39 UTC
[PR] KAFKA-200135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
m1a2st opened a new pull request, #21484:
URL: https://github.com/apache/kafka/pull/21484
Some public api classes are missing java document, we should add the comment
--
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
Re: [PR] KAFKA-200135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on code in PR #21484:
URL: https://github.com/apache/kafka/pull/21484#discussion_r2819801604
##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1229,7 +1229,7 @@ default ListPartitionReassignmentsResult listPartitionReassignments(
}
/**
- * List all of the current partition reassignments
+ * List all the current partition reassignments
Review Comment:
```suggestion
* List all the current partition reassignments.
```
##########
clients/src/main/java/org/apache/kafka/clients/admin/AlterConfigOp.java:
##########
@@ -76,10 +79,18 @@ public enum OpType {
this.id = id;
}
+ /**
+ * Returns the byte ID associated with this operation type. This ID is used for serialization
+ * and protocol communication.
+ */
public byte id() {
return id;
}
+ /**
+ * Returns the {@code OpType} corresponding to the given byte ID, or {@code null} if the
+ * ID does not match any known operation type.
Review Comment:
Interesting that it returns `null` as opposed to throwing an `IllegalArgumentException` like many of the other string/integer -> enum methods do 🤔
##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1250,6 +1250,8 @@ default ListPartitionReassignmentsResult listPartitionReassignments(ListPartitio
}
/**
+ * List all the current partition reassignments
Review Comment:
Ditto as above.
##########
clients/src/main/java/org/apache/kafka/clients/admin/ClassicGroupDescription.java:
##########
@@ -91,23 +122,21 @@ public String groupId() {
}
/**
- * The group protocol type.
+ * The group protocol type, or the empty string.
Review Comment:
It doesn't look like there's anything preventing a `null` from being passed to the constructor for `protocol`. `protocolData` is forced to an empty string, but `protocol` is not 🤔
##########
clients/src/main/java/org/apache/kafka/clients/admin/ClassicGroupDescription.java:
##########
@@ -91,23 +122,21 @@ public String groupId() {
}
/**
- * The group protocol type.
+ * The group protocol type, or the empty string.
*/
public String protocol() {
return protocol;
}
/**
- * The group protocol data. The meaning depends on the group protocol type.
- * For a classic consumer group, this is the partition assignor name.
- * For a classic connect group, this indicates which Connect protocols are enabled.
+ * The group protocol data.
Review Comment:
Is the detail in the previous description for this method incorrect now?
##########
clients/src/main/java/org/apache/kafka/clients/admin/AlterConfigOp.java:
##########
@@ -76,10 +79,18 @@ public enum OpType {
this.id = id;
}
+ /**
+ * Returns the byte ID associated with this operation type. This ID is used for serialization
Review Comment:
We should be consistent, if possible. Either choose `id` or `ID`, preferably the latter.
--
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
Re: [PR] KAFKA-200135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #21484:
URL: https://github.com/apache/kafka/pull/21484#discussion_r2820478496
##########
clients/src/main/java/org/apache/kafka/clients/admin/ClassicGroupDescription.java:
##########
@@ -91,23 +122,21 @@ public String groupId() {
}
/**
- * The group protocol type.
+ * The group protocol type, or the empty string.
*/
public String protocol() {
return protocol;
}
/**
- * The group protocol data. The meaning depends on the group protocol type.
- * For a classic consumer group, this is the partition assignor name.
- * For a classic connect group, this indicates which Connect protocols are enabled.
+ * The group protocol data.
Review Comment:
I moved these comments to the class level, but I think it’s also fine to keep them here. Therefore, I will revert this change.
--
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
Re: [PR] KAFKA-200135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #21484:
URL: https://github.com/apache/kafka/pull/21484#discussion_r2820469429
##########
clients/src/main/java/org/apache/kafka/clients/admin/AlterConfigOp.java:
##########
@@ -76,10 +79,18 @@ public enum OpType {
this.id = id;
}
+ /**
+ * Returns the byte ID associated with this operation type. This ID is used for serialization
+ * and protocol communication.
+ */
public byte id() {
return id;
}
+ /**
+ * Returns the {@code OpType} corresponding to the given byte ID, or {@code null} if the
+ * ID does not match any known operation type.
Review Comment:
It will return `null`, since this method retrieves values from the `OP_TYPES` map. If the key does not exist, the returned value will be `null`.
--
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
Re: [PR] KAFKA-200135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on code in PR #21484:
URL: https://github.com/apache/kafka/pull/21484#discussion_r2908580145
##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1250,6 +1250,8 @@ default ListPartitionReassignmentsResult listPartitionReassignments(ListPartitio
}
/**
+ * List all the current partition reassignments
+ *
* @param partitions the partitions we want to get reassignment for, or an empty optional if we want to get the reassignments for all partitions in the cluster
Review Comment:
We're capitalizing the start of the sentences for the parameters. We should be consistent. Also we may want to spit this line in two.
##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1250,6 +1250,8 @@ default ListPartitionReassignmentsResult listPartitionReassignments(ListPartitio
}
/**
+ * List all the current partition reassignments
Review Comment:
Seems like we want to end this sentence with a period ('.').
##########
clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java:
##########
@@ -308,14 +354,25 @@ protected AdminClientConfig(Map<?, ?> props, boolean doLog) {
super(CONFIG, props, doLog);
}
+ /**
+ * Get the set of configuration names supported by this configuration class.
+ * @return the set of configuration names.
Review Comment:
Inconsistent capitalization.
```suggestion
* @return The set of configuration names.
```
##########
clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java:
##########
@@ -39,7 +39,14 @@
import static org.apache.kafka.common.config.ConfigDef.ValidString.in;
/**
- * The AdminClient configuration class, which also contains constants for configuration entry names.
+ * Configuration for the {@link Admin} client. This class defines all configuration properties
+ * accepted by the admin client and provides constants for their names.
+ * <p>
+ * At minimum, {@link #BOOTSTRAP_SERVERS_CONFIG} must be set to connect to brokers, or
Review Comment:
```suggestion
* At minimum, <em>either</em> {@link #BOOTSTRAP_SERVERS_CONFIG} must be set to connect to brokers, or
```
--
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
Re: [PR] KAFKA-20135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21484:
URL: https://github.com/apache/kafka/pull/21484#issuecomment-4884872474
This PR is being marked as stale since it has not had any activity in 90 days. If you
would like to keep this PR alive, please leave a comment asking for a review. If the PR has
merge conflicts, update it with the latest from the base branch.
<p>
If you are having difficulty finding a reviewer, please reach out on the
[mailing list](https://kafka.apache.org/contact).
<p>
If this PR is no longer valid or desired, please feel free to close it. If no activity
occurs in the next 30 days, it will be automatically closed.
--
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
Re: [PR] KAFKA-200135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #21484:
URL: https://github.com/apache/kafka/pull/21484#discussion_r2820517730
##########
clients/src/main/java/org/apache/kafka/clients/admin/ClassicGroupDescription.java:
##########
@@ -91,23 +122,21 @@ public String groupId() {
}
/**
- * The group protocol type.
+ * The group protocol type, or the empty string.
Review Comment:
When I traced the code path for setting the protocol type, I found that it is translated as `group.protocolType().orElse("")`. so null is prevented at the call site.
--
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
Re: [PR] KAFKA-20135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #21484: KAFKA-20135 Add javadoc to org.apache.kafka.clients.admin module(1/N)
URL: https://github.com/apache/kafka/pull/21484
--
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
Re: [PR] KAFKA-20135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #21484:
URL: https://github.com/apache/kafka/pull/21484#issuecomment-4031402553
Thanks for @kirktrue review, I have addressed all comments
--
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
Re: [PR] KAFKA-20135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21484:
URL: https://github.com/apache/kafka/pull/21484#issuecomment-5739065282
This PR has been closed since it has not had any activity in 120 days. If you feel like this
was a mistake, or you would like to continue working on it, please feel free to re-open the
PR and ask for a 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
Re: [PR] KAFKA-200135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on PR #21484:
URL: https://github.com/apache/kafka/pull/21484#issuecomment-4027740395
Oh, and the PR description contains a typo in the Jira ID. It should be `KAFKA-20135` instead of `KAFKA-200135`.
--
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
Re: [PR] KAFKA-20135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #21484:
URL: https://github.com/apache/kafka/pull/21484#issuecomment-4188590396
Hello @kirktrue, Can we continue moving forward with 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