[jira] [Commented] (KAFKA-7446) Better error message to explain the upper limit of TimeWindow
[
https://issues.apache.org/jira/browse/KAFKA-7446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16708277#comment-16708277
]
ASF GitHub Bot commented on KAFKA-7446:
---
guozhangwang closed pull request #5930: KAFKA-7446: Fix the duration and
instant validation messages
URL: https://github.com/apache/kafka/pull/5930
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
b/streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
index 819732ac6d8..fdeb8845611 100644
--- a/streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
+++ b/streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
@@ -82,6 +82,7 @@
import static org.apache.kafka.common.utils.Utils.getHost;
import static org.apache.kafka.common.utils.Utils.getPort;
+import static
org.apache.kafka.streams.internals.ApiUtils.prepareMillisCheckFailMsgPrefix;
/**
* A Kafka client that allows for performing continuous computation on input
coming from one or more input topics and
@@ -919,7 +920,8 @@ public void run() {
* @throws IllegalArgumentException if {@code timeout} can't be
represented as {@code long milliseconds}
*/
public synchronized boolean close(final Duration timeout) throws
IllegalArgumentException {
-ApiUtils.validateMillisecondDuration(timeout, "timeout");
+final String msgPrefix = prepareMillisCheckFailMsgPrefix(timeout,
"timeout");
+ApiUtils.validateMillisecondDuration(timeout, msgPrefix);
final long timeoutMs = timeout.toMillis();
if (timeoutMs < 0) {
diff --git
a/streams/src/main/java/org/apache/kafka/streams/internals/ApiUtils.java
b/streams/src/main/java/org/apache/kafka/streams/internals/ApiUtils.java
index e888d7a120b..dd3b691b10c 100644
--- a/streams/src/main/java/org/apache/kafka/streams/internals/ApiUtils.java
+++ b/streams/src/main/java/org/apache/kafka/streams/internals/ApiUtils.java
@@ -18,43 +18,57 @@
import java.time.Duration;
import java.time.Instant;
-import java.util.Objects;
+
+import static java.lang.String.format;
public final class ApiUtils {
+
+private static final String MILLISECOND_VALIDATION_FAIL_MSG_FRMT =
"Invalid value for parameter \"%s\" (value was: %s). ";
+
private ApiUtils() {
}
/**
* Validates that milliseconds from {@code duration} can be retrieved.
* @param duration Duration to check.
- * @param name Name of params for an error message.
+ * @param messagePrefix Prefix text for an error message.
* @return Milliseconds from {@code duration}.
*/
-public static long validateMillisecondDuration(final Duration duration,
final String name) {
+public static long validateMillisecondDuration(final Duration duration,
final String messagePrefix) {
try {
if (duration == null)
-throw new IllegalArgumentException("[" +
Objects.toString(name) + "] shouldn't be null.");
+throw new IllegalArgumentException(messagePrefix + "It
shouldn't be null.");
return duration.toMillis();
} catch (final ArithmeticException e) {
-throw new IllegalArgumentException("[" + name + "] can't be
converted to milliseconds. ", e);
+throw new IllegalArgumentException(messagePrefix + "It can't be
converted to milliseconds.", e);
}
}
/**
* Validates that milliseconds from {@code instant} can be retrieved.
* @param instant Instant to check.
- * @param name Name of params for an error message.
+ * @param messagePrefix Prefix text for an error message.
* @return Milliseconds from {@code instant}.
*/
-public static long validateMillisecondInstant(final Instant instant, final
String name) {
+public static long validateMillisecondInstant(final Instant instant, final
String messagePrefix) {
try {
if (instant == null)
-throw new IllegalArgumentException("[" + name + "] shouldn't
be null.");
+throw new IllegalArgumentException(messagePrefix + "It
shouldn't be null.");
return instant.toEpochMilli();
} catch (final ArithmeticException e) {
-throw new IllegalArgumentException("[" + name + "] can't be
converted to milliseconds. ", e);
+throw new IllegalArgumentException(messagePrefix + "It can't be
converted to milliseconds.", e);
}
}
+
+/**
+ * Generates the prefix message for validateMillisecondXX() utility
+ * @param value Object to be converted to milliseconds
+ * @param name Object name
+ * @retur
[jira] [Commented] (KAFKA-7446) Better error message to explain the upper limit of TimeWindow
[
https://issues.apache.org/jira/browse/KAFKA-7446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16692077#comment-16692077
]
ASF GitHub Bot commented on KAFKA-7446:
---
mrsrinivas opened a new pull request #5930: [WIP] KAFKA-7446: Better error
messages on `WindowDuration` and `AdvanceInterval`
URL: https://github.com/apache/kafka/pull/5930
Changes made as part of this PR
- Added new `Exception` classes for `WindowDuration` and `AdvanceInterval`
- Improved error message for better readability
- Corrected java documentation on `AdvanceInterval` check.
- Improved JUnit tests with `expected` attribute in `@Test`
Core and Streams modules' JUnit tests are successful in local but
tests related to file system is failed since I am running on Windows.
### 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 GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Better error message to explain the upper limit of TimeWindow
> -
>
> Key: KAFKA-7446
> URL: https://issues.apache.org/jira/browse/KAFKA-7446
> Project: Kafka
> Issue Type: Improvement
> Components: streams
>Affects Versions: 2.0.0
>Reporter: Jacek Laskowski
>Assignee: Srinivas Reddy
>Priority: Trivial
> Labels: newbie++
>
> The following code throws a {{IllegalArgumentException}}.
> {code:java}
> import org.apache.kafka.streams.kstream.TimeWindows
> import scala.concurrent.duration._
> val timeWindow = TimeWindows
> .of(1.minute.toMillis)
> .advanceBy(2.minutes.toMillis)
> {code}
> The exception is as follows and it's not clear why {{6}} is the upper
> limit (not to mention that {{AdvanceMs}} with the uppercase {{A}} did also
> confuse me).
> {code:java}
> java.lang.IllegalArgumentException: AdvanceMs must lie within interval (0,
> 6].
> at
> org.apache.kafka.streams.kstream.TimeWindows.advanceBy(TimeWindows.java:100)
> ... 44 elided{code}
> I think that the message should be more developer-friendly and explain the
> boundaries, perhaps with an example (and a link to docs)?
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (KAFKA-7446) Better error message to explain the upper limit of TimeWindow
[
https://issues.apache.org/jira/browse/KAFKA-7446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16690134#comment-16690134
]
Guozhang Wang commented on KAFKA-7446:
--
[~mrsrinivas] Please feel free to submit the PR.
> Better error message to explain the upper limit of TimeWindow
> -
>
> Key: KAFKA-7446
> URL: https://issues.apache.org/jira/browse/KAFKA-7446
> Project: Kafka
> Issue Type: Improvement
> Components: streams
>Affects Versions: 2.0.0
>Reporter: Jacek Laskowski
>Assignee: Srinivas Reddy
>Priority: Trivial
> Labels: newbie++
>
> The following code throws a {{IllegalArgumentException}}.
> {code:java}
> import org.apache.kafka.streams.kstream.TimeWindows
> import scala.concurrent.duration._
> val timeWindow = TimeWindows
> .of(1.minute.toMillis)
> .advanceBy(2.minutes.toMillis)
> {code}
> The exception is as follows and it's not clear why {{6}} is the upper
> limit (not to mention that {{AdvanceMs}} with the uppercase {{A}} did also
> confuse me).
> {code:java}
> java.lang.IllegalArgumentException: AdvanceMs must lie within interval (0,
> 6].
> at
> org.apache.kafka.streams.kstream.TimeWindows.advanceBy(TimeWindows.java:100)
> ... 44 elided{code}
> I think that the message should be more developer-friendly and explain the
> boundaries, perhaps with an example (and a link to docs)?
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (KAFKA-7446) Better error message to explain the upper limit of TimeWindow
[
https://issues.apache.org/jira/browse/KAFKA-7446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16685102#comment-16685102
]
Srinivas Reddy commented on KAFKA-7446:
---
Hi [~jlaskowski], Are you working on this? Should I pick if not?
> Better error message to explain the upper limit of TimeWindow
> -
>
> Key: KAFKA-7446
> URL: https://issues.apache.org/jira/browse/KAFKA-7446
> Project: Kafka
> Issue Type: Improvement
> Components: streams
>Affects Versions: 2.0.0
>Reporter: Jacek Laskowski
>Priority: Trivial
> Labels: newbie++
>
> The following code throws a {{IllegalArgumentException}}.
> {code:java}
> import org.apache.kafka.streams.kstream.TimeWindows
> import scala.concurrent.duration._
> val timeWindow = TimeWindows
> .of(1.minute.toMillis)
> .advanceBy(2.minutes.toMillis)
> {code}
> The exception is as follows and it's not clear why {{6}} is the upper
> limit (not to mention that {{AdvanceMs}} with the uppercase {{A}} did also
> confuse me).
> {code:java}
> java.lang.IllegalArgumentException: AdvanceMs must lie within interval (0,
> 6].
> at
> org.apache.kafka.streams.kstream.TimeWindows.advanceBy(TimeWindows.java:100)
> ... 44 elided{code}
> I think that the message should be more developer-friendly and explain the
> boundaries, perhaps with an example (and a link to docs)?
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
