Cron schedule with offset #188
Conversation
honnix
requested review from
kumare3,
matthewphsmith and
wild-endeavor
as code owners
| @@ -12,7 +12,6 @@ coverage==5.3 # via -r dev-requirements.in | |||
| flake8-black==0.2.1 # via -r dev-requirements.in | |||
| flake8-isort==4.0.0 # via -r dev-requirements.in | |||
| flake8==3.8.3 # via -r dev-requirements.in, flake8-black, flake8-isort | |||
| importlib-metadata==1.7.0 # via -c requirements.txt, flake8, pluggy, pytest | |||
There was a problem hiding this comment.
I ran pip-compile dev-requirements.in.
There was a problem hiding this comment.
Maybe because I ran it under Python 3.8? I can revert the change if seems risky.
| @@ -7,30 +7,31 @@ | |||
| -e file:.#egg=flytekit # via -r requirements.in | |||
| ansiwrap==0.8.4 # via papermill | |||
| appdirs==1.4.4 # via black | |||
| appnope==0.1.0 # via ipykernel, ipython | |||
There was a problem hiding this comment.
I ran pip-compile requirements.in.
There was a problem hiding this comment.
same as if you run make requirements right?
There was a problem hiding this comment.
Interesting. Actually not. First of all, I had to fix Makefile because of _install-piptools that should be install-piptools; then requirements.txt file header became:
#
# This file is autogenerated by pip-compile
# To update, run:
#
# make requirements.txt
#
I will make a commit to reflect the difference.
There was a problem hiding this comment.
I see only a few things been upgraded because of define PIP_COMPILE.
|
@wild-endeavor I think this is ready for review. Please take a look and check whether this change would break backward compatibility in some way. Thanks a lot. |
This reverts commit cd5568a.
honnix
force-pushed
the
cron-schedule-with-offset
branch
from
a1b28a9 to
25d4ef8
Compare
|
Thanks a lot for approving this! |
* Extend `CronSchedule` to support `schedule` and `offset` added in flyteorg/flyteidl#83 Co-authored-by: tnsetting <[email protected]>
TL;DR
Support validate and propagate CronScheduleWithOffset.
Type
Are all requirements met?
Complete description
Motivation: Why do you think this is important?
When integrating with our scheduler Styx, we will need to extend the semantics of schedule to support Styx cron syntax, cron aliases, as well as offset.
Goal: What should the final outcome look like, ideally?
Scheduleprotobuf definition is extended to support Styx style schedule.
Describe alternatives you've considered
We tried reusing existing cron but the syntax is a bit different and flytekit doesn't support Styx style cron aliases; and more importantly, we need to support offset which is one the most important concepts in Styx.
More details can be found in this issue.
Related and depended change in flyteidl is flyteorg/flyteidl#83.
Tracking Issue
flyteorg/flyte#529
Follow-up issue