From 1f20051634d20f0512139268bff0a24ef7b70938 Mon Sep 17 00:00:00 2001 From: Alex Wu Date: Thu, 14 Aug 2025 11:15:23 +0800 Subject: [PATCH 1/3] feature: add cron date check in flyte schedule Signed-off-by: Alex Wu --- flytekit/core/schedule.py | 11 ++++++++- tests/flytekit/unit/core/test_schedule.py | 30 +++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/flytekit/core/schedule.py b/flytekit/core/schedule.py index d9bb621cf0..f1c32ddcdb 100644 --- a/flytekit/core/schedule.py +++ b/flytekit/core/schedule.py @@ -142,12 +142,21 @@ def _validate_expression(cron_expression: str): def _validate_schedule(schedule: str): if schedule.lower() not in CronSchedule._VALID_CRON_ALIASES: try: - croniter.croniter(schedule) + cron = croniter.croniter(schedule) except Exception: raise ValueError( "Schedule is invalid. It must be set to either a cron alias or valid cron expression." f" Provided schedule: {schedule}" ) + # Check if the cron expression can actually produce valid dates + try: + # Try to get the next occurrence to validate the schedule + cron.get_next(datetime.datetime) + except Exception as e: + raise ValueError( + f"Schedule contains invalid date combinations." + f"Provided schedule: {schedule}. Error: {str(e)}" + ) @staticmethod def _validate_offset(offset: str): diff --git a/tests/flytekit/unit/core/test_schedule.py b/tests/flytekit/unit/core/test_schedule.py index c66c93d646..735b845306 100644 --- a/tests/flytekit/unit/core/test_schedule.py +++ b/tests/flytekit/unit/core/test_schedule.py @@ -153,3 +153,33 @@ def quadruple(a: int) -> int: assert lp.schedule == _schedule_models.Schedule( "kickoff_input", rate=_schedule_models.Schedule.FixedRate(12, _schedule_models.Schedule.FixedRateUnit.HOUR) ) + + +def test_cron_invalid_date_combinations(): + """Test that CronSchedule rejects invalid date combinations like 31st of February.""" + + # Test invalid date combinations + invalid_schedules = [ + "0 0 31 2 *", # February 31st (does not exist) + "0 0 30 2 *", # February 30th (does not exist) + "0 0 31 4 *", # April 31st (does not exist) + "0 0 31 6 *", # June 31st (does not exist) + ] + + for invalid_schedule in invalid_schedules: + with pytest.raises(ValueError, match="Schedule contains invalid date combinations"): + CronSchedule(schedule=invalid_schedule) + + # Test valid date combinations that should pass + valid_schedules = [ + "0 0 28 2 *", # February 28th (always valid) + "0 0 29 2 *", # February 29th (valid in leap years - handled by croniter) + "0 0 30 4 *", # April 30th (valid) + "0 0 31 1 *", # January 31st (valid) + "0 0 31 3 *", # March 31st (valid) + ] + + for valid_schedule in valid_schedules: + # These should not raise any exceptions + obj = CronSchedule(schedule=valid_schedule) + assert obj.cron_schedule.schedule == valid_schedule From d0d401f77ab02e92cec1fa4a84a53e415ee9d590 Mon Sep 17 00:00:00 2001 From: Alex Wu Date: Thu, 14 Aug 2025 11:16:20 +0800 Subject: [PATCH 2/3] fix: lint fix Signed-off-by: Alex Wu --- flytekit/core/schedule.py | 3 +-- tests/flytekit/unit/core/test_schedule.py | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/flytekit/core/schedule.py b/flytekit/core/schedule.py index f1c32ddcdb..4d421a6a19 100644 --- a/flytekit/core/schedule.py +++ b/flytekit/core/schedule.py @@ -154,8 +154,7 @@ def _validate_schedule(schedule: str): cron.get_next(datetime.datetime) except Exception as e: raise ValueError( - f"Schedule contains invalid date combinations." - f"Provided schedule: {schedule}. Error: {str(e)}" + f"Schedule contains invalid date combinations." f"Provided schedule: {schedule}. Error: {str(e)}" ) @staticmethod diff --git a/tests/flytekit/unit/core/test_schedule.py b/tests/flytekit/unit/core/test_schedule.py index 735b845306..7574cbf247 100644 --- a/tests/flytekit/unit/core/test_schedule.py +++ b/tests/flytekit/unit/core/test_schedule.py @@ -157,7 +157,7 @@ def quadruple(a: int) -> int: def test_cron_invalid_date_combinations(): """Test that CronSchedule rejects invalid date combinations like 31st of February.""" - + # Test invalid date combinations invalid_schedules = [ "0 0 31 2 *", # February 31st (does not exist) @@ -165,11 +165,11 @@ def test_cron_invalid_date_combinations(): "0 0 31 4 *", # April 31st (does not exist) "0 0 31 6 *", # June 31st (does not exist) ] - + for invalid_schedule in invalid_schedules: with pytest.raises(ValueError, match="Schedule contains invalid date combinations"): CronSchedule(schedule=invalid_schedule) - + # Test valid date combinations that should pass valid_schedules = [ "0 0 28 2 *", # February 28th (always valid) @@ -178,7 +178,7 @@ def test_cron_invalid_date_combinations(): "0 0 31 1 *", # January 31st (valid) "0 0 31 3 *", # March 31st (valid) ] - + for valid_schedule in valid_schedules: # These should not raise any exceptions obj = CronSchedule(schedule=valid_schedule) From 4faae880aee33546f84c9972141242dbbca5abb7 Mon Sep 17 00:00:00 2001 From: Alex Wu Date: Fri, 15 Aug 2025 10:03:35 +0800 Subject: [PATCH 3/3] fix: refactor code structure Signed-off-by: Alex Wu --- flytekit/core/schedule.py | 12 ++------ tests/flytekit/unit/core/test_schedule.py | 36 ++++++++++++----------- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/flytekit/core/schedule.py b/flytekit/core/schedule.py index 4d421a6a19..98c7929cad 100644 --- a/flytekit/core/schedule.py +++ b/flytekit/core/schedule.py @@ -142,20 +142,12 @@ def _validate_expression(cron_expression: str): def _validate_schedule(schedule: str): if schedule.lower() not in CronSchedule._VALID_CRON_ALIASES: try: + # Validate the cron expression cron = croniter.croniter(schedule) - except Exception: - raise ValueError( - "Schedule is invalid. It must be set to either a cron alias or valid cron expression." - f" Provided schedule: {schedule}" - ) - # Check if the cron expression can actually produce valid dates - try: # Try to get the next occurrence to validate the schedule cron.get_next(datetime.datetime) except Exception as e: - raise ValueError( - f"Schedule contains invalid date combinations." f"Provided schedule: {schedule}. Error: {str(e)}" - ) + raise ValueError(f"Schedule is invalid. Provided schedule: {schedule} Error: {str(e)}") @staticmethod def _validate_offset(offset: str): diff --git a/tests/flytekit/unit/core/test_schedule.py b/tests/flytekit/unit/core/test_schedule.py index 7574cbf247..bcb6ad03fa 100644 --- a/tests/flytekit/unit/core/test_schedule.py +++ b/tests/flytekit/unit/core/test_schedule.py @@ -155,31 +155,33 @@ def quadruple(a: int) -> int: ) -def test_cron_invalid_date_combinations(): - """Test that CronSchedule rejects invalid date combinations like 31st of February.""" - - # Test invalid date combinations - invalid_schedules = [ +@pytest.mark.parametrize( + "invalid_schedule", + [ "0 0 31 2 *", # February 31st (does not exist) "0 0 30 2 *", # February 30th (does not exist) "0 0 31 4 *", # April 31st (does not exist) "0 0 31 6 *", # June 31st (does not exist) - ] + ], +) +def test_cron_invalid_date_combinations(invalid_schedule): + """Test that CronSchedule rejects invalid date combinations like 31st of February.""" + with pytest.raises(ValueError, match="Schedule is invalid."): + CronSchedule(schedule=invalid_schedule) - for invalid_schedule in invalid_schedules: - with pytest.raises(ValueError, match="Schedule contains invalid date combinations"): - CronSchedule(schedule=invalid_schedule) - # Test valid date combinations that should pass - valid_schedules = [ +@pytest.mark.parametrize( + "valid_schedule", + [ "0 0 28 2 *", # February 28th (always valid) "0 0 29 2 *", # February 29th (valid in leap years - handled by croniter) "0 0 30 4 *", # April 30th (valid) "0 0 31 1 *", # January 31st (valid) "0 0 31 3 *", # March 31st (valid) - ] - - for valid_schedule in valid_schedules: - # These should not raise any exceptions - obj = CronSchedule(schedule=valid_schedule) - assert obj.cron_schedule.schedule == valid_schedule + ], +) +def test_cron_valid_date_combinations(valid_schedule): + """Test that CronSchedule accepts valid date combinations.""" + # These should not raise any exceptions + obj = CronSchedule(schedule=valid_schedule) + assert obj.cron_schedule.schedule == valid_schedule