raminqaf commented on code in PR #27132:
URL: https://github.com/apache/flink/pull/27132#discussion_r2444519329
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/IntervalFreshnessUtils.java:
##########
@@ -42,35 +37,28 @@ public class IntervalFreshnessUtils {
private IntervalFreshnessUtils() {}
- @VisibleForTesting
- static void validateIntervalFreshness(IntervalFreshness intervalFreshness)
{
- if (!NumberUtils.isParsable(intervalFreshness.getInterval())) {
- throw new ValidationException(
- String.format(
- "The interval freshness value '%s' is an illegal
integer type value.",
- intervalFreshness.getInterval()));
- }
-
- if (!NumberUtils.isDigits(intervalFreshness.getInterval())) {
- throw new ValidationException(
- "The freshness interval currently only supports integer
type values.");
- }
- }
-
- public static Duration convertFreshnessToDuration(IntervalFreshness
intervalFreshness) {
- // validate the freshness value firstly
- validateIntervalFreshness(intervalFreshness);
-
- long interval = Long.parseLong(intervalFreshness.getInterval());
+ /**
+ * Validates that the given freshness can be converted to a cron
expression in full refresh
+ * mode. Since freshness and cron expression cannot be converted
equivalently, there are
+ * currently only a limited patterns of freshness that are supported.
+ *
+ * @param intervalFreshness the freshness to validate
+ * @throws ValidationException if the freshness cannot be converted to a
valid cron expression
+ */
+ public static void validateFreshnessForCron(IntervalFreshness
intervalFreshness) {
switch (intervalFreshness.getTimeUnit()) {
- case DAY:
- return Duration.ofDays(interval);
- case HOUR:
- return Duration.ofHours(interval);
- case MINUTE:
- return Duration.ofMinutes(interval);
case SECOND:
- return Duration.ofSeconds(interval);
+ validateCronConstraints(intervalFreshness,
SECOND_CRON_UPPER_BOUND);
+ break;
+ case MINUTE:
+ validateCronConstraints(intervalFreshness,
MINUTE_CRON_UPPER_BOUND);
+ break;
+ case HOUR:
+ validateCronConstraints(intervalFreshness,
HOUR_CRON_UPPER_BOUND);
+ break;
+ case DAY:
+ validateDayConstraints(intervalFreshness);
+ break;
Review Comment:
I think the reason this validation is here, its because it is used in the
internal API. I can move this to the IntervalFreshness class and get rid of the
IntervalFreshnessUtils entirely.
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/IntervalFreshnessUtils.java:
##########
@@ -42,35 +37,28 @@ public class IntervalFreshnessUtils {
private IntervalFreshnessUtils() {}
- @VisibleForTesting
- static void validateIntervalFreshness(IntervalFreshness intervalFreshness)
{
- if (!NumberUtils.isParsable(intervalFreshness.getInterval())) {
- throw new ValidationException(
- String.format(
- "The interval freshness value '%s' is an illegal
integer type value.",
- intervalFreshness.getInterval()));
- }
-
- if (!NumberUtils.isDigits(intervalFreshness.getInterval())) {
- throw new ValidationException(
- "The freshness interval currently only supports integer
type values.");
- }
- }
-
- public static Duration convertFreshnessToDuration(IntervalFreshness
intervalFreshness) {
- // validate the freshness value firstly
- validateIntervalFreshness(intervalFreshness);
-
- long interval = Long.parseLong(intervalFreshness.getInterval());
+ /**
+ * Validates that the given freshness can be converted to a cron
expression in full refresh
+ * mode. Since freshness and cron expression cannot be converted
equivalently, there are
+ * currently only a limited patterns of freshness that are supported.
+ *
+ * @param intervalFreshness the freshness to validate
+ * @throws ValidationException if the freshness cannot be converted to a
valid cron expression
+ */
+ public static void validateFreshnessForCron(IntervalFreshness
intervalFreshness) {
switch (intervalFreshness.getTimeUnit()) {
- case DAY:
- return Duration.ofDays(interval);
- case HOUR:
- return Duration.ofHours(interval);
- case MINUTE:
- return Duration.ofMinutes(interval);
case SECOND:
- return Duration.ofSeconds(interval);
+ validateCronConstraints(intervalFreshness,
SECOND_CRON_UPPER_BOUND);
+ break;
+ case MINUTE:
+ validateCronConstraints(intervalFreshness,
MINUTE_CRON_UPPER_BOUND);
+ break;
+ case HOUR:
+ validateCronConstraints(intervalFreshness,
HOUR_CRON_UPPER_BOUND);
+ break;
+ case DAY:
+ validateDayConstraints(intervalFreshness);
+ break;
Review Comment:
I think the reason this validation is here, its because it is used in the
internal API. I can move this to the IntervalFreshness class and get rid of the
IntervalFreshnessUtils entirely. WDYT?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
