1996fanrui commented on code in PR #22560:
URL: https://github.com/apache/flink/pull/22560#discussion_r1189688192


##########
flink-core/src/test/java/org/apache/flink/util/TimeUtilsTest.java:
##########
@@ -118,6 +118,12 @@ public void testParseDurationTrim() {
         assertEquals(155L, TimeUtils.parseDuration("      155      ms   
").toMillis());
     }
 
+    @Test
+    public void testParseDurationWithNegativeNumber() {
+        assertEquals(-1L, TimeUtils.parseDuration("-1ms").toMillis());
+        assertEquals(-1L, TimeUtils.parseDuration("-1").toMillis());

Review Comment:
   Could you add some cases with multiple `-`?



##########
flink-core/src/main/java/org/apache/flink/util/TimeUtils.java:
##########
@@ -69,7 +69,8 @@ public static Duration parseDuration(String text) {
         int pos = 0;
 
         char current;
-        while (pos < len && (current = trimmed.charAt(pos)) >= '0' && current 
<= '9') {
+        // Negative number is supported for special usage, such as -1ms for 
execution.buffer-timeout
+        while (pos < len && ((current = trimmed.charAt(pos)) >= '0' && current 
<= '9' || current == '-')) {

Review Comment:
   We should check that the `-` must be the first char, right?



-- 
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: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to