Re: [PR] MINOR: Server-Commons cleanup [kafka]
jlprat merged PR #14572: URL: https://github.com/apache/kafka/pull/14572 -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: Server-Commons cleanup [kafka]
jlprat commented on PR #14572: URL: https://github.com/apache/kafka/pull/14572#issuecomment-1773257357 All tests seem to be unrelated to the 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: Server-Commons cleanup [kafka]
jlprat commented on PR #14572: URL: https://github.com/apache/kafka/pull/14572#issuecomment-1772862994 I mistakenly added 2 imports in the last commit (added when writing the example code) -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: Server-Commons cleanup [kafka]
jlprat commented on code in PR #14572:
URL: https://github.com/apache/kafka/pull/14572#discussion_r1366996892
##
server-common/src/main/java/org/apache/kafka/server/util/json/JsonValue.java:
##
@@ -26,18 +26,16 @@
/**
* A simple wrapper over Jackson's JsonNode that enables type safe parsing via
the `DecodeJson` type
* class.
- *
+ *
* Typical usage would be something like:
- *
- * {{{
+ *
Review Comment:
Done! Thanks for the feedback @mimaison !
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] MINOR: Server-Commons cleanup [kafka]
jlprat commented on code in PR #14572:
URL: https://github.com/apache/kafka/pull/14572#discussion_r1366961639
##
server-common/src/main/java/org/apache/kafka/server/util/json/JsonValue.java:
##
@@ -26,18 +26,16 @@
/**
* A simple wrapper over Jackson's JsonNode that enables type safe parsing via
the `DecodeJson` type
* class.
- *
+ *
* Typical usage would be something like:
- *
- * {{{
+ *
Review Comment:
Oh good catch! I'll update this and the other feedback
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] MINOR: Server-Commons cleanup [kafka]
mimaison commented on code in PR #14572:
URL: https://github.com/apache/kafka/pull/14572#discussion_r1366957220
##
server-common/src/main/java/org/apache/kafka/server/util/json/JsonValue.java:
##
@@ -26,18 +26,16 @@
/**
* A simple wrapper over Jackson's JsonNode that enables type safe parsing via
the `DecodeJson` type
* class.
- *
+ *
* Typical usage would be something like:
- *
- * {{{
+ *
Review Comment:
It looks like we forgot to update the code example when we converted this
class to Java, the snippet below is still in Scala.
##
server-common/src/main/java/org/apache/kafka/server/util/timer/Timer.java:
##
@@ -27,7 +27,7 @@ public interface Timer extends AutoCloseable {
/**
* Advance the internal clock, executing any tasks whose expiration has
been
* reached within the duration of the passed timeout.
- * @param timeoutMs
+ * @param timeoutMs the tme to advance in milliseconds
Review Comment:
`tme` -> `time`
##
server-common/src/main/java/org/apache/kafka/server/util/ThroughputThrottler.java:
##
@@ -64,7 +64,7 @@ public ThroughputThrottler(long targetThroughput, long
startMs) {
* @param amountSoFar bytes produced so far if you want to throttle data
throughput, or
*messages produced so far if you want to throttle
message throughput.
* @param sendStartMs timestamp of the most recently sent message
- * @return
+ * @return true if throttling happened
Review Comment:
Should it be `true if throttling should happen`?
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] MINOR: Server-Commons cleanup [kafka]
jlprat commented on PR #14572: URL: https://github.com/apache/kafka/pull/14572#issuecomment-1772555347 Hi @mimaison you were doing some similar changes recently, if you have some time, I'd highly appreciate 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: Server-Commons cleanup [kafka]
jlprat commented on code in PR #14572:
URL: https://github.com/apache/kafka/pull/14572#discussion_r1363669588
##
server-common/src/main/java/org/apache/kafka/timeline/BaseHashTable.java:
##
@@ -224,15 +224,15 @@ static int findSlot(Object object, int numElements) {
*/
static void unpackSlot(List out, Object[] elements, int slot) {
Object value = elements[slot];
-if (value == null) {
-return;
Review Comment:
This is done to avoid a return on a void method
##
server-common/src/main/java/org/apache/kafka/queue/KafkaEventQueue.java:
##
@@ -201,7 +201,7 @@ private void remove(EventContext eventContext) {
}
}
-private void handleEvents() throws InterruptedException {
Review Comment:
No call in the method throws `InterruptedException`
##
server-common/src/main/java/org/apache/kafka/timeline/BaseHashTable.java:
##
@@ -180,7 +180,7 @@ final T baseRemove(Object key) {
/**
* Expand the hash table to a new size. Existing elements will be copied
to new slots.
*/
-final private void rehash(int newSize) {
Review Comment:
This is not needed
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
