feat(cssc): 31005729 support cron expressions for '--schedule' parameter #8
Conversation
There was a problem hiding this comment.
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/acrcssc/azext_acrcssc/tests/latest/test_validators.py:38
- Additional test cases for invalid cron expressions should be added to cover different invalid cron patterns.
test_cases = [('df'),('12'),('dd'),('41d'), ('21dd')]
| return | ||
|
|
||
| # Validate cron expression using croniter | ||
| if not croniter(schedule).is_valid: |
There was a problem hiding this comment.
We restrict the minimum interval to 1 day when using n day format. Shall we enforce a minimum interval for the cron format? This would ensure consistency with the "n day" format and help prevent triggering excessive runs in a short period, which could increase the load and cause throttling.
There was a problem hiding this comment.
what should be the minimum interval, if we make it 'at least' once a day (match something like [0-9]{1-2} [0-9]{1-2} * * *) then end up in the same place as using the schedule option, no?
What additional flexibility do we want to add by adding the usage of cron expressions but not allowing the user to specify it?
User can would still be able to modify the trigger task by hand and do whatever setup they want and we cannot block that.
…edule, centralize regex to match daily schedule
|
Moving to 'draft' as this is not targeted for the current semester. |
Add cron expression support for the
--scheduleparameter and update the help and error texts.Validation of the cron expression is done via croniter and submitted to the Task API when creating/updating a workflow task.