st2 icon indicating copy to clipboard operation
st2 copied to clipboard

Add a new cron function

Open chain312 opened this issue 2 years ago • 9 comments

Problem description

image When I used core.st2.CronTimer to create a scheduled task, I found that I could not specify the start time and end time

Solution

As can be seen from the source code, the scheduled task is implemented by aspscheduler and the parameters of the scheduled task are transparently transmitted. aspscheduler has this function. In use, parameters can be specified as start_date and end_date. You only need to change CRON_PARAMETERS_SCHEMA to add these two parameters

chain312 avatar Oct 16 '22 09:10 chain312

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Oct 16 '22 09:10 CLAassistant

set the milestone

chain312 avatar Oct 17 '22 11:10 chain312

Thanks for looking into this enhancement, and working out how it can be achieved. It's great to get new contributions.

The CronTrigger is really supposed to be mimicing the functionality of a cron job, and with a cron job you don't really get start and end times. However, as you mention https://apscheduler.readthedocs.io/en/3.x/modules/triggers/cron.html does support start and end dates.

It would be useful to do a post on the StackStorm development slack channels to see what people think of this enhancement request, as I'm not sure if its best to start a new timer type rather than extend the cron timer. We can extend the crontimer as you have mentioned, but that's not something that is in cron itself, and ties ourself to using apscheduler. So, I'd like to get the view of some others in the TSC about whether this is the right place to put it. But, as this functionality is not available anywhere else it would be good functionality to extend StackStorm, and this would be the simplest way of adding it.

If we do go the route you've coded, then the only addition I think this needs is some unit-tests added. There are a variety of tests on CronTimer with invalid formats etc in ./st2api/tests/unit/controllers/v1/test_rules.py, so it would be good to get these extended with some valid and invalid examples that use the start_date and end_date parameters.

amanda11 avatar Oct 17 '22 14:10 amanda11

Thanks for looking into this enhancement, and working out how it can be achieved. It's great to get new contributions.

The CronTrigger is really supposed to be mimicing the functionality of a cron job, and with a cron job you don't really get start and end times. However, as you mention https://apscheduler.readthedocs.io/en/3.x/modules/triggers/cron.html does support start and end dates.

It would be useful to do a post on the StackStorm development slack channels to see what people think of this enhancement request, as I'm not sure if its best to start a new timer type rather than extend the cron timer. We can extend the crontimer as you have mentioned, but that's not something that is in cron itself, and ties ourself to using apscheduler.

I agree with @amanda11 here. It might make sense for it to be a new timer type (core.st2.ScheduledCronTimer?), so that the new functionality can be kept separate and minimize changes, if any, for other users.

With that said, I would love to have this included and can try to get this rolling for the next release.

rush-skills avatar Jan 11 '23 13:01 rush-skills

Thanks for looking into this enhancement, and working out how it can be achieved. It's great to get new contributions. The CronTrigger is really supposed to be mimicing the functionality of a cron job, and with a cron job you don't really get start and end times. However, as you mention https://apscheduler.readthedocs.io/en/3.x/modules/triggers/cron.html does support start and end dates. It would be useful to do a post on the StackStorm development slack channels to see what people think of this enhancement request, as I'm not sure if its best to start a new timer type rather than extend the cron timer. We can extend the crontimer as you have mentioned, but that's not something that is in cron itself, and ties ourself to using apscheduler.

I agree with @amanda11 here. It might make sense for it to be a new timer type (core.st2.ScheduledCronTimer?), so that the new functionality can be kept separate and minimize changes, if any, for other users.

With that said, I would love to have this included and can try to get this rolling for the next release.

@rush-skills I posted a discussion of this feature in my slack channel, but no one talked to me about it. This feature can be used to start a scheduled task at a certain time of day. For example, I started a vulnerability scan task after 12 o 'clock on January 12. I also think we could add a new type of trigger.

chain312 avatar Jan 11 '23 13:01 chain312

Thanks for looking into this enhancement, and working out how it can be achieved. It's great to get new contributions. The CronTrigger is really supposed to be mimicing the functionality of a cron job, and with a cron job you don't really get start and end times. However, as you mention https://apscheduler.readthedocs.io/en/3.x/modules/triggers/cron.html does support start and end dates. It would be useful to do a post on the StackStorm development slack channels to see what people think of this enhancement request, as I'm not sure if its best to start a new timer type rather than extend the cron timer. We can extend the crontimer as you have mentioned, but that's not something that is in cron itself, and ties ourself to using apscheduler.

I agree with @amanda11 here. It might make sense for it to be a new timer type (core.st2.ScheduledCronTimer?), so that the new functionality can be kept separate and minimize changes, if any, for other users.

With that said, I would love to have this included and can try to get this rolling for the next release.

@rush-skills I saw the problem and thought we could create a trigger that exposed all the aspscheduler parametersst2 cron timer was missed

chain312 avatar Jan 11 '23 13:01 chain312

@rush-skills I saw the problem and thought we could create a trigger that exposed all the aspscheduler parametersst2 cron timer was missed

What is the default behaviour when the start_date or end_date is not specified? Does it work like the old version and thus retains backward compatibility? In that case, I would be happy to merge this.

But I think some tests should be added if possible to check the above. @amanda11 Do you know if there is a way to test this?

rush-skills avatar Feb 07 '23 12:02 rush-skills

st2common/unit/test_trigger_services.py is where there are some unit-tests for the crontimer, so if keeping as the same trigger and to prove backwards compatiblity. We should have some tests in there with and without the start/end time. It uses test rules that are setup in: st2tests/st2tests/fixtures/generic/rules, so those could be expanded.

I couldn't find any integration tests that check times actually go off when you expect...

amanda11 avatar Feb 07 '23 13:02 amanda11

st2common/unit/test_trigger_services.py is where there are some unit-tests for the crontimer, so if keeping as the same trigger and to prove backwards compatiblity. We should have some tests in there with and without the start/end time. It uses test rules that are setup in: st2tests/st2tests/fixtures/generic/rules, so those could be expanded.

I couldn't find any integration tests that check times actually go off when you expect... @rush-skills I'll write test cases later when I have time

chain312 avatar Feb 14 '23 02:02 chain312