[BUG]Invalid cron date schedule creates infinite loop in flytescheduler

[popojk]

Tracking issue

Related to #6470

Why are the changes needed?

In the Flytekit SDK, it is proposed that an error be raised if the user enters a cron job with an invalid date (e.g., 0 0 31 2 *), preventing the schedule from being sent to the backend.

What changes were proposed in this pull request?

Add schedule date validation check in flytekit SDK.

How was this patch tested?

Test workflow:

from datetime import datetime

from flytekit import task, workflow, CronSchedule, LaunchPlan


@task
def task_1(message: str) -> str:
    return message


@workflow
def date_formatter_wf2() -> str:
    return task_1("hello")

cron_lp = LaunchPlan.get_or_create(
    name="cron_scheduled_lp",
    workflow=date_formatter_wf2,
    schedule=CronSchedule(
        schedule="0 0 31 2 *",
    ),
)

After registering the launch_plan with invalid schedule date, the SDK throws error as expected.

Check all the applicable boxes

• All new and existing tests passed.
• All commits are signed-off.

Summary by Bito

This pull request fixes a bug in the Flytekit SDK related to invalid cron date schedules that could cause an infinite loop by adding validation for cron schedules. It introduces error handling for invalid cron expressions and includes comprehensive tests to ensure correct behavior for both valid and invalid inputs, improving the scheduling feature's robustness.

[machichima]

Can we combine this check with above so that we do not need to have two try...except block

[machichima]

btw is there any .validate() function that we can use instead of using get_next()?

[JiangJiaWei1103]

I suggest keeping them separate, and here's why:

1. The first try block checks syntax: Whether schedule is a valid cron expression
2. The second try block checks semantics: Whether the expression can generate valid dates

If they're combined together, the error handling would become generic and make it harder for users to pinpoint the actual problem.

@machichima, WDYT?

[machichima]

Then how about adding Error: {str(e)} in the end of the error message?

While those two try...expect are just alike, I would still prefer merging them together. But I agree that we should print the error out here so that we know the actual problem

[JiangJiaWei1103]

Sounds reasonable, let's merge them together and throw out the error message so users can understand whether it's a syntax error or semantic one.

[popojk]

@machichima @JiangJiaWei1103 Thank you both for the advice. I've merged two try..except blocks into one.

[popojk]

btw is there any .validate() function that we can use instead of using get_next()?

@machichima I think the only way for croniter to validate cron date is calling get_next function. I'm not sure whether we can do the validation by others cron library, yet I prefer to keep using croniter as it is a simple way.

[machichima]

Consider using @pytest.parametrize() to represent multiple test cases?

[codecov]

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.15%. Comparing base (eb5a67f) to head (4faae88).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
flytekit/core/schedule.py	0.00%	4 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (eb5a67f) and HEAD (4faae88). Click for more details.

HEAD has 92 uploads less than BASE

Flag	BASE (eb5a67f)	HEAD (4faae88)
	100	8

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #3312       +/-   ##
===========================================
- Coverage   85.26%   41.15%   -44.11%     
===========================================
  Files         386      250      -136     
  Lines       30276    25038     -5238     
  Branches     2969     2950       -19     
===========================================
- Hits        25814    10304    -15510     
- Misses       3615    14631    +11016     
+ Partials      847      103      -744     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[machichima]

LGTM

cc @pingsutw