phet commented on code in PR #3742:
URL: https://github.com/apache/gobblin/pull/3742#discussion_r1296248584
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -291,6 +291,10 @@ private boolean addSpecHelperMethod(Spec spec) {
*/
@VisibleForTesting
public static boolean isWithinRange(String cronExpression, int
maxNumDaysToScheduleWithin) {
+ if (cronExpression == null || cronExpression.trim().isEmpty()) {
+ // If the cron expression is empty or null, return true to capture adhoc
flows
+ return true;
+ }
Review Comment:
at the core, I understand why we don't continue w/ `cronExpression ==
null`... but is this really happening? more canonical would be to have callers
avoid calling this w/ `null`. then add enforcement here via `Preconditions`
that croaks if `null` is passed in.
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -291,6 +291,10 @@ private boolean addSpecHelperMethod(Spec spec) {
*/
@VisibleForTesting
public static boolean isWithinRange(String cronExpression, int
maxNumDaysToScheduleWithin) {
+ if (cronExpression == null || cronExpression.trim().isEmpty()) {
+ // If the cron expression is empty or null, return true to capture adhoc
flows
+ return true;
+ }
Review Comment:
at the core, I understand why we don't continue w/ `cronExpression ==
null`... but is this really happening? more canonical would be to have callers
avoid calling this w/ `null`. then add enforcement here via `Preconditions`
that croaks when `null` is passed in.
--
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]
