TisonKun commented on a change in pull request #9686: [FLINK-14069] [core]
Enable TimeUtils for all time units labels supported by scala Duration
URL: https://github.com/apache/flink/pull/9686#discussion_r325039730
##########
File path: flink-core/src/main/java/org/apache/flink/util/TimeUtils.java
##########
@@ -64,64 +82,71 @@ public static Duration parseDuration(String text) {
"' cannot be re represented as 64bit number
(numeric overflow).");
}
- final long multiplier;
- if (unit.isEmpty()) {
- multiplier = 1L;
- } else {
- if (matchTimeUnit(unit, TimeUnit.MILLISECONDS)) {
- multiplier = 1L;
- } else if (matchTimeUnit(unit, TimeUnit.SECONDS)) {
- multiplier = 1000L;
- } else if (matchTimeUnit(unit, TimeUnit.MINUTES)) {
- multiplier = 1000L * 60L;
- } else if (matchTimeUnit(unit, TimeUnit.HOURS)) {
- multiplier = 1000L * 60L * 60L;
- } else {
- throw new IllegalArgumentException("Time
interval unit '" + unit +
- "' does not match any of the recognized
units: " + TimeUnit.getAllUnits());
- }
+ if (unitLabel.isEmpty()) {
+ return Duration.of(value, ChronoUnit.MILLIS);
}
- final long result = value * multiplier;
-
- // check for overflow
- if (result / multiplier != value) {
- throw new IllegalArgumentException("The value '" + text
+
- "' cannot be re represented as 64bit number of
bytes (numeric overflow).");
+ ChronoUnit unit = LABEL_TO_UNIT_MAP.get(unitLabel);
+ if (unit != null) {
+ return Duration.of(value, unit);
+ } else {
+ throw new IllegalArgumentException("Time interval unit
label '" + unitLabel +
+ "' does not match any of the recognized units:
" + TimeUnit.getAllUnits());
}
-
- return Duration.ofMillis(result);
}
- private static boolean matchTimeUnit(String text, TimeUnit unit) {
- return text.equals(unit.getUnit());
+ private static Map<String, ChronoUnit> initMap() {
+ Map<String, ChronoUnit> labelToUnit = new HashMap<>();
+ for (TimeUnit timeUnit : TimeUnit.values()) {
+ for (String label : timeUnit.getLabels()) {
+ labelToUnit.put(label, timeUnit.getUnit());
+ }
+ }
+ return labelToUnit;
}
/**
* Enum which defines time unit, mostly used to parse value from
configuration file.
*/
private enum TimeUnit {
- MILLISECONDS("ms"),
- SECONDS("s"),
- MINUTES("min"),
- HOURS("h");
- private String unit;
+ DAYS(ChronoUnit.DAYS, "d", "day"),
+ HOURS(ChronoUnit.HOURS, "h", "hour"),
+ MINUTES(ChronoUnit.MINUTES, "min", "minute"),
+ SECONDS(ChronoUnit.SECONDS, "s", "sec", "second"),
+ MILLISECONDS(ChronoUnit.MILLIS, "ms", "milli", "millisecond"),
+ MICROSECONDS(ChronoUnit.MICROS, "µs", "micro", "microsecond"),
+ NANOSECONDS(ChronoUnit.NANOS, "ns", "nano", "nanosecond");
+
+ private String[] labels;
+
+ private ChronoUnit unit;
+
+ TimeUnit(ChronoUnit unit, String... labels) {
+ this.unit = checkNotNull(unit);
Review comment:
nit: `TimeUnit` has the only private constructor, it is ok to reduce code
complexity by directly assign without runtime checker.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
