Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Flytescheduler cron expression validation process #4953

Open
2 tasks done
Tracked by #5783
BroderPeters opened this issue Feb 26, 2024 · 2 comments
Open
2 tasks done
Tracked by #5783

[BUG] Flytescheduler cron expression validation process #4953

BroderPeters opened this issue Feb 26, 2024 · 2 comments
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working documentation Improvements or additions to documentation flyteadmin Issue for FlyteAdmin Service hacktoberfest

Comments

@BroderPeters
Copy link

Describe the bug

Cron expressions like 0 6 * * MON#1 are currently able to being registered, display and executed, but flytescheduler produces the following error logs:

unable to register the schedule {BaseModel:{ID:1259 CreatedAt:2024-02-05 09:51:51.390164 +0000 UTC UpdatedAt:2024-02-05 09:51:51.390164 +0000 UTC DeletedAt:<nil>} SchedulableEntityKey:{Project:<project> Domain:development Name:<my>-launchplan Version:3622cca2ce009e394c1d193d43b6fe46c062afdc} CronExpression:0 6 * * MON#1 FixedRateValue:0 Unit:MINUTE KickoffTimeInputArg: Active:0xc000a042b4} due to failed to parse int from MON#1: strconv.Atoi: parsing "MON#1": invalid syntax

Which according to the flyte docs should also be the expected behavior, as the # symbol is not listed here. Generally, # is used for the nth occurrence of a day of a week in a month.

From a discussion in the community slack

Expected behavior

Either cron expression that don't fit the allowed symbols from the docs are properly validated while registering the launch plan or expressions with symbols like # are also supported in the flytescheduler validation.

Additional context to reproduce

Used Launchplan:

LaunchPlan.get_or_create(
    name="my_launchplan",
    workflow=wf,
    schedule=CronSchedule(
        schedule="0 6 * * MON#1",  # UTC, https://docs.flyte.org/en/latest/concepts/schedules.html#cron-expression-table
    )
)

Screenshots

image
image

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@BroderPeters BroderPeters added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Feb 26, 2024
Copy link

welcome bot commented Feb 26, 2024

Thank you for opening your first issue here! 🛠

Copy link

dosubot bot commented Feb 26, 2024

🤖

Hey @BroderPeters, back again to keep us on our toes, huh? 😄 Always a pleasure seeing your name pop up here!

The error you're encountering is likely due to the fact that the cron expression 0 6 * * MON#1 is not supported by the cron parser used in Flyte. The # symbol is not a standard cron expression and is not mentioned in the cron implementation described in the comments of the CronSchedule message in the schedule.proto file.

The cron implementation used by Flyte is based on the standard cron as described by Wikipedia and also supports nonstandard predefined scheduling definitions as described by Amazon CloudWatch, except @reboot.

To resolve this issue, you have two options:

  1. Modify your cron expression to fit the supported format. For example, if you want to schedule a job for the first Monday of every month at 6:00, you could use a cron expression like 0 6 1-7 * MON. This will schedule the job to run at 6:00 on Monday, but only if the date is between the 1st and the 7th, effectively ensuring that it runs on the first Monday of the month.

  2. If you need to use the # symbol or other non-standard cron expressions, you would need to modify the cron parser used by Flyte to support these expressions. This would involve changing the source code of Flyte and may not be feasible depending on your situation.

Please note that any changes to the cron parser would need to be thoroughly tested to ensure they don't introduce any new issues. You can find more information about the cron parser used by Flyte in the schedule.proto file.

Sources

About Dosu

This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@dosubot dosubot bot added the documentation Improvements or additions to documentation label Feb 26, 2024
@pingsutw pingsutw added flyteadmin Issue for FlyteAdmin Service and removed untriaged This issues has not yet been looked at by the Maintainers labels Feb 29, 2024
@eapolinario eapolinario added the backlogged For internal use. Reserved for contributor team workflow. label Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working documentation Improvements or additions to documentation flyteadmin Issue for FlyteAdmin Service hacktoberfest
Projects
None yet
Development

No branches or pull requests

4 participants