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.

Sign up for GitHub

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

Jump to bottom

Adds MAX_RRULE_LENGTH #7762

Merged
merged 8 commits into from
Jan 6, 2023
Merged

Adds MAX_RRULE_LENGTH #7762

merged 8 commits into from
Jan 6, 2023

Conversation

jakekaplan
Copy link
Contributor

@jakekaplan jakekaplan commented Dec 5, 2022 •
edited
Loading

Adds MAX_RRULE_LENGTH

Currently it is possible to submit an arbitrary length json as an rrule for a Deployment.schedule using rdates. This can lead to timeouts as the giant json needs to be serialized multiple times going from the database to the client. While the schedules are technically valid this can be abused past what the server can reasonably support. This provides a "reasonable maximum length" (6210 is approximately a year of rdates + some buffer = 6500) for rrules as a guardrail.

closes: https://github.com/PrefectHQ/nebula/issues/2738

Example

start = datetime(2000, 1, 1)
s = "RDATE:" + ",".join(
    [start.add(days=i).format("YMMDD") + "T000000Z" for i in range(1000)]
)
RRuleSchedule(rrule=s)
Traceback (most recent call last):
  File "/Users/jakekaplan/PycharmProjects/prefect/jake-threading.py", line 9, in <module>
    RRuleSchedule(rrule=s)
  File "pydantic/main.py", line 342, in pydantic.main.BaseModel.__init__
pydantic.error_wrappers.ValidationError: 1 validation error for RRuleSchedule
rrule
  Invalid RRule string "RDATE:20000101T000000Z,20000102T000000Z,20000103T000000Z,20000104T000000Z,20000105T000000Z,20000106T000000Z,20000107T000000Z,20000108T000000Z,20000109T000000Z,20000110T000000Z,20000111T000000Z,20000112T000000Z,20000113T000000Z,20000114T000000Z,20000115T000000Z,20000116T000000Z,20000117T000000Z,20000118T000000Z,20000119T000000Z,20000120T000000Z,20000121T000000Z,20000122T000000Z,20000123T000000Z,20000124T000000Z,20000125T000000Z,20000126T000000Z,20000127T000000Z,20000128T000000Z,20000129T000000Z,20000130T000000Z,20000131T000000Z,20000201T000000Z,20000202T000000Z,20000203T000000Z,20000204T000000Z,20000205T000000Z,20000206T000000Z,20000207T000000Z,20000208T000000Z,20000209T000000Z,20000210T000000Z,20000211T000000Z,20000212T000000Z,20000213T000000Z,20000214T000000Z,20000215T000000Z,20000216T000000Z,20000217T000000Z,20000218T000000Z,20000219T000000Z,20000220T000000Z,20000221T000000Z,20000222T000000Z,20000223T000000Z,20000224T000000Z,20000225T000000Z,20000226T000000Z,20000227T000000Z,20000228T000000Z,20000229T000000Z,20000301T000000Z,20000302T000000Z,20000303T000000Z,20000304T000000Z,20000305T000000Z,20000306T000000Z,20000307T000000Z,20000308T000000Z,20000309T000000Z,20000310T000000Z,20000311T000000Z,20000312T000000Z,20000313T000000Z,20000314T000000Z,20000315T000000Z,20000316T000000Z,20000317T000000Z,20000318T000000Z,20000319T000000Z,20000320T000000Z,20000321T000000Z,20000322T000000Z,20000323T000000Z,20000324T000000Z,20000325T000000Z,20000326T000000Z,20000327T000000Z,20000328T000000Z,20000329T000000Z,20000330T000000Z,20000331T000000Z,20000401T000000Z,20000402T000000Z,20000403T000000Z,20000404T000000Z,20000405T000000Z,20000406T000000Z,20000407T000000Z,20000408T000000Z,20000409T000000Z,20000410T000000Z,20000411T000000Z,20000412T000000Z,20000413T000000Z,20000414T000000Z,20000415T000000Z,20000416T000000Z,20000417T000000Z,20000418T000000Z,20000419T000000Z,20000420T000000Z,20000421T000000Z,20000422T000000Z,20000423T000000Z,20000424T000000Z,20000425T000000Z,20000426T000000Z,20000427T000000Z,20000428T000000Z,20000429T000000Z,20000430T000000Z,20000501T000000Z,20000502T000000Z,20000503T000000Z,20000504T000000Z,20000505T000000Z,20000506T000000Z,20000507T000000Z,20000508T000000Z,20000509T000000Z,20000510T000000Z,20000511T000000Z,20000512T000000Z,20000513T000000Z,20000514T000000Z,20000515T000000Z,20000516T000000Z,20000517T000000Z,20000518T000000Z,20000519T000000Z,20000520T000000Z,20000521T000000Z,20000522T000000Z,20000523T000000Z,20000524T000000Z,20000525T000000Z,20000526T000000Z,20000527T000000Z,20000528T000000Z,20000529T000000Z,20000530T000000Z,20000531T000000Z,20000601T000000Z,20000602T000000Z,20000603T000000Z,20000604T000000Z,20000605T000000Z,20000606T000000Z,20000607T000000Z,20000608T000000Z,20000609T000000Z,20000610T000000Z,20000611T000000Z,20000612T000000Z,20000613T000000Z,20000614T000000Z,20000615T000000Z,20000616T000000Z,20000617T000000Z,20000618T000000Z,20000619T000000Z,20000620T000000Z,20000621T000000Z,20000622T000000Z,20000623T000000Z,20000624T000000Z,20000625T000000Z,20000626T000000Z,20000627T000000Z,20000628T000000Z,20000629T000000Z,20000630T000000Z,20000701T000000Z,20000702T000000Z,20000703T000000Z,20000704T000000Z,20000705T000000Z,20000706T000000Z,20000707T000000Z,20000708T000000Z,20000709T000000Z,20000710T000000Z,20000711T000000Z,20000712T000000Z,20000713T000000Z,20000714T000000Z,20000715T000000Z,20000716T000000Z,20000717T000000Z,20000718T000000Z,20000719T000000Z,20000720T000000Z,20000721T000000Z,20000722T000000Z,20000723T000000Z,20000724T000000Z,20000725T000000Z,20000726T000000Z,20000727T000000Z,20000728T000000Z,20000729T000000Z,20000730T000000Z,20000731T000000Z,20000801T000000Z,20000802T000000Z,20000803T000000Z,20000804T000000Z,20000805T000000Z,20000806T000000Z,20000807T000000Z,20000808T000000Z,20000809T000000Z,20000810T000000Z,20000811T000000Z,20000812T000000Z,20000813T000000Z,20000814T000000Z,20000815T000000Z,20000816T000000Z,20000817T000000Z,20000818T000000Z,20000819T000000Z,20000820T000000Z,20000821T000000Z,20000822T000000Z,20000823T000000Z,20000824T000000Z,20000825T000000Z,20000826T000000Z,20000827T000000Z,20000828T000000Z,20000829T000000Z,20000830T000000Z,20000831T000000Z,20000901T000000Z,20000902T000000Z,20000903T000000Z,20000904T000000Z,20000905T000000Z,20000906T000000Z,20000907T000000Z,20000908T000000Z,20000909T000000Z,20000910T000000Z,20000911T000000Z,20000912T000000Z,20000913T000000Z,20000914T000000Z,20000915T000000Z,20000916T000000Z,20000917T000000Z,20000918T000000Z,20000919T000000Z,20000920T000000Z,20000921T000000Z,20000922T000000Z,20000923T000000Z,20000924T000000Z,20000925T000000Z,20000926T000000Z,20000927T000000Z,20000928T000000Z,20000929T000000Z,20000930T000000Z,20001001T000000Z,20001002T000000Z,20001003T000000Z,20001004T000000Z,20001005T000000Z,20001006T000000Z,20001007T000000Z,20001008T000000Z,20001009T000000Z,20001010T000000Z,20001011T000000Z,20001012T000000Z,20001013T000000Z,20001014T000000Z,20001015T000000Z,20001016T000000Z,20001017T000000Z,20001018T000000Z,20001019T000000Z,20001020T000000Z,20001021T000000Z,20001022T000000Z,20001023T000000Z,20001024T000000Z,20001025T000000Z,20001026T000000Z,20001027T000000Z,20001028T000000Z,20001029T000000Z,20001030T000000Z,20001031T000000Z,20001101T000000Z,20001102T000000Z,20001103T000000Z,20001104T000000Z,20001105T000000Z,20001106T000000Z,20001107T000000Z,20001108T000000Z,20001109T000000Z,20001110T000000Z,20001111T000000Z,20001112T000000Z,20001113T000000Z,20001114T000000Z,20001115T000000Z,20001116T000000Z,20001117T000000Z,20001118T000000Z,20001119T000000Z,20001120T000000Z,20001121T000000Z,20001122T000000Z,20001123T000000Z,20001124T000000Z,20001125T000000Z,20001126T000000Z,20001127T000000Z,20001128T000000Z,20001129T000000Z,20001130T000000Z,20001201T000000Z,20001202T000000Z,20001203T000000Z,20001204T000000Z,20001205T000000Z,20001206T000000Z,20001207T000000Z,20001208T000000Z,20001209T000000Z,20001210T000000Z,20001211T000000Z,20001212T000000Z,20001213T000000Z,20001214T000000Z,20001215T000000Z,20001216T000000Z,20001217T000000Z,20001218T000000Z,20001219T000000Z,20001220T000000Z,20001221T000000Z,20001222T000000Z,20001223T000000Z,20001224T000000Z,20001225T000000Z,20001226T000000Z,20001227T000000Z,20001228T000000Z,20001229T000000Z,20001230T000000Z,20001231T000000Z,20010101T000000Z,20010102T000000Z,20010103T000000Z,20010104T000000Z,20010105T000000Z,20010106T000000Z,20010107T000000Z,20010108T000000Z,20010109T000000Z,20010110T000000Z,20010111T000000Z,20010112T000000Z,20010113T000000Z,20010114T000000Z,20010115T000000Z,20010116T000000Z,20010117T000000Z,20010118T000000Z,20010119T000000Z,20010120T000000Z,20010121T000000Z,20010122T000000Z,20010123T000000Z,20010124T000000Z,20010125T000000Z,20010126T000000Z,20010127T000000Z,20010128T000000Z,20010129T000000Z,20010130T000000Z,20010131T000000Z,20010201T000000Z,20010202T000000Z,20010203T000000Z,20010204T000000Z,20010205T000000Z,20010206T000000Z,20010207T000000Z,20010208T000000Z,20010209T000000Z,20010210T000000Z,20010211T000000Z,20010212T000000Z,20010213T000000Z,20010214T000000Z,20010215T000000Z,20010216T000000Z,20010217T000000Z,20010218T000000Z,20010219T000000Z,20010220T000000Z,20010221T000000Z,20010222T000000Z,20010223T000000Z,20010224T000000Z,20010225T000000Z,20010226T000000Z,20010227T000000Z,20010228T000000Z,20010301T000000Z,20010302T000000Z,20010303T000000Z,20010304T000000Z,20010305T000000Z,20010306T000000Z,20010307T000000Z,20010308T000000Z,20010309T000000Z,20010310T000000Z,20010311T000000Z,20010312T000000Z,20010313T000000Z,20010314T000000Z,20010315T000000Z,20010316T000000Z,20010317T000000Z,20010318T000000Z,20010319T000000Z,20010320T000000Z,20010321T000000Z,20010322T000000Z,20010323T000000Z,20010324T000000Z,20010325T000000Z,20010326T000000Z,20010327T000000Z,20010328T000000Z,20010329T000000Z,20010330T000000Z,20010331T000000Z,20010401T000000Z,20010402T000000Z,20010403T000000Z,20010404T000000Z,20010405T000000Z,20010406T000000Z,20010407T000000Z,20010408T000000Z,20010409T000000Z,20010410T000000Z,20010411T000000Z,20010412T000000Z,20010413T000000Z,20010414T000000Z,20010415T000000Z,20010416T000000Z,20010417T000000Z,20010418T000000Z,20010419T000000Z,20010420T000000Z,20010421T000000Z,20010422T000000Z,20010423T000000Z,20010424T000000Z,20010425T000000Z,20010426T000000Z,20010427T000000Z,20010428T000000Z,20010429T000000Z,20010430T000000Z,20010501T000000Z,20010502T000000Z,20010503T000000Z,20010504T000000Z,20010505T000000Z,20010506T000000Z,20010507T000000Z,20010508T000000Z,20010509T000000Z,20010510T000000Z,20010511T000000Z,20010512T000000Z,20010513T000000Z,20010514T000000Z,20010515T000000Z,20010516T000000Z,20010517T000000Z,20010518T000000Z,20010519T000000Z,20010520T000000Z,20010521T000000Z,20010522T000000Z,20010523T000000Z,20010524T000000Z,20010525T000000Z,20010526T000000Z,20010527T000000Z,20010528T000000Z,20010529T000000Z,20010530T000000Z,20010531T000000Z,20010601T000000Z,20010602T000000Z,20010603T000000Z,20010604T000000Z,20010605T000000Z,20010606T000000Z,20010607T000000Z,20010608T000000Z,20010609T000000Z,20010610T000000Z,20010611T000000Z,20010612T000000Z,20010613T000000Z,20010614T000000Z,20010615T000000Z,20010616T000000Z,20010617T000000Z,20010618T000000Z,20010619T000000Z,20010620T000000Z,20010621T000000Z,20010622T000000Z,20010623T000000Z,20010624T000000Z,20010625T000000Z,20010626T000000Z,20010627T000000Z,20010628T000000Z,20010629T000000Z,20010630T000000Z,20010701T000000Z,20010702T000000Z,20010703T000000Z,20010704T000000Z,20010705T000000Z,20010706T000000Z,20010707T000000Z,20010708T000000Z,20010709T000000Z,20010710T000000Z,20010711T000000Z,20010712T000000Z,20010713T000000Z,20010714T000000Z,20010715T000000Z,20010716T000000Z,20010717T000000Z,20010718T000000Z,20010719T000000Z,20010720T000000Z,20010721T000000Z,20010722T000000Z,20010723T000000Z,20010724T000000Z,20010725T000000Z,20010726T000000Z,20010727T000000Z,20010728T000000Z,20010729T000000Z,20010730T000000Z,20010731T000000Z,20010801T000000Z,20010802T000000Z,20010803T000000Z,20010804T000000Z,20010805T000000Z,20010806T000000Z,20010807T000000Z,20010808T000000Z,20010809T000000Z,20010810T000000Z,20010811T000000Z,20010812T000000Z,20010813T000000Z,20010814T000000Z,20010815T000000Z,20010816T000000Z,20010817T000000Z,20010818T000000Z,20010819T000000Z,20010820T000000Z,20010821T000000Z,20010822T000000Z,20010823T000000Z,20010824T000000Z,20010825T000000Z,20010826T000000Z,20010827T000000Z,20010828T000000Z,20010829T000000Z,20010830T000000Z,20010831T000000Z,20010901T000000Z,20010902T000000Z,20010903T000000Z,20010904T000000Z,20010905T000000Z,20010906T000000Z,20010907T000000Z,20010908T000000Z,20010909T000000Z,20010910T000000Z,20010911T000000Z,20010912T000000Z,20010913T000000Z,20010914T000000Z,20010915T000000Z,20010916T000000Z,20010917T000000Z,20010918T000000Z,20010919T000000Z,20010920T000000Z,20010921T000000Z,20010922T000000Z,20010923T000000Z,20010924T000000Z,20010925T000000Z,20010926T000000Z,20010927T000000Z,20010928T000000Z,20010929T000000Z,20010930T000000Z,20011001T000000Z,20011002T000000Z,20011003T000000Z,20011004T000000Z,20011005T000000Z,20011006T000000Z,20011007T000000Z,20011008T000000Z,20011009T000000Z,20011010T000000Z,20011011T000000Z,20011012T000000Z,20011013T000000Z,20011014T000000Z,20011015T000000Z,20011016T000000Z,20011017T000000Z,20011018T000000Z,20011019T000000Z,20011020T000000Z,20011021T000000Z,20011022T000000Z,20011023T000000Z,20011024T000000Z,20011025T000000Z,20011026T000000Z,20011027T000000Z,20011028T000000Z,20011029T000000Z,20011030T000000Z,20011031T000000Z,20011101T000000Z,20011102T000000Z,20011103T000000Z,20011104T000000Z,20011105T000000Z,20011106T000000Z,20011107T000000Z,20011108T000000Z,20011109T000000Z,20011110T000000Z,20011111T000000Z,20011112T000000Z,20011113T000000Z,20011114T000000Z,20011115T000000Z,20011116T000000Z,20011117T000000Z,20011118T000000Z,20011119T000000Z,20011120T000000Z,20011121T000000Z,20011122T000000Z,20011123T000000Z,20011124T000000Z,20011125T000000Z,20011126T000000Z,20011127T000000Z,20011128T000000Z,20011129T000000Z,20011130T000000Z,20011201T000000Z,20011202T000000Z,20011203T000000Z,20011204T000000Z,20011205T000000Z,20011206T000000Z,20011207T000000Z,20011208T000000Z,20011209T000000Z,20011210T000000Z,20011211T000000Z,20011212T000000Z,20011213T000000Z,20011214T000000Z,20011215T000000Z,20011216T000000Z,20011217T000000Z,20011218T000000Z,20011219T000000Z,20011220T000000Z,20011221T000000Z,20011222T000000Z,20011223T000000Z,20011224T000000Z,20011225T000000Z,20011226T000000Z,20011227T000000Z,20011228T000000Z,20011229T000000Z,20011230T000000Z,20011231T000000Z,20020101T000000Z,20020102T000000Z,20020103T000000Z,20020104T000000Z,20020105T000000Z,20020106T000000Z,20020107T000000Z,20020108T000000Z,20020109T000000Z,20020110T000000Z,20020111T000000Z,20020112T000000Z,20020113T000000Z,20020114T000000Z,20020115T000000Z,20020116T000000Z,20020117T000000Z,20020118T000000Z,20020119T000000Z,20020120T000000Z,20020121T000000Z,20020122T000000Z,20020123T000000Z,20020124T000000Z,20020125T000000Z,20020126T000000Z,20020127T000000Z,20020128T000000Z,20020129T000000Z,20020130T000000Z,20020131T000000Z,20020201T000000Z,20020202T000000Z,20020203T000000Z,20020204T000000Z,20020205T000000Z,20020206T000000Z,20020207T000000Z,20020208T000000Z,20020209T000000Z,20020210T000000Z,20020211T000000Z,20020212T000000Z,20020213T000000Z,20020214T000000Z,20020215T000000Z,20020216T000000Z,20020217T000000Z,20020218T000000Z,20020219T000000Z,20020220T000000Z,20020221T000000Z,20020222T000000Z,20020223T000000Z,20020224T000000Z,20020225T000000Z,20020226T000000Z,20020227T000000Z,20020228T000000Z,20020301T000000Z,20020302T000000Z,20020303T000000Z,20020304T000000Z,20020305T000000Z,20020306T000000Z,20020307T000000Z,20020308T000000Z,20020309T000000Z,20020310T000000Z,20020311T000000Z,20020312T000000Z,20020313T000000Z,20020314T000000Z,20020315T000000Z,20020316T000000Z,20020317T000000Z,20020318T000000Z,20020319T000000Z,20020320T000000Z,20020321T000000Z,20020322T000000Z,20020323T000000Z,20020324T000000Z,20020325T000000Z,20020326T000000Z,20020327T000000Z,20020328T000000Z,20020329T000000Z,20020330T000000Z,20020331T000000Z,20020401T000000Z,20020402T000000Z,20020403T000000Z,20020404T000000Z,20020405T000000Z,20020406T000000Z,20020407T000000Z,20020408T000000Z,20020409T000000Z,20020410T000000Z,20020411T000000Z,20020412T000000Z,20020413T000000Z,20020414T000000Z,20020415T000000Z,20020416T000000Z,20020417T000000Z,20020418T000000Z,20020419T000000Z,20020420T000000Z,20020421T000000Z,20020422T000000Z,20020423T000000Z,20020424T000000Z,20020425T000000Z,20020426T000000Z,20020427T000000Z,20020428T000000Z,20020429T000000Z,20020430T000000Z,20020501T000000Z,20020502T000000Z,20020503T000000Z,20020504T000000Z,20020505T000000Z,20020506T000000Z,20020507T000000Z,20020508T000000Z,20020509T000000Z,20020510T000000Z,20020511T000000Z,20020512T000000Z,20020513T000000Z,20020514T000000Z,20020515T000000Z,20020516T000000Z,20020517T000000Z,20020518T000000Z,20020519T000000Z,20020520T000000Z,20020521T000000Z,20020522T000000Z,20020523T000000Z,20020524T000000Z,20020525T000000Z,20020526T000000Z,20020527T000000Z,20020528T000000Z,20020529T000000Z,20020530T000000Z,20020531T000000Z,20020601T000000Z,20020602T000000Z,20020603T000000Z,20020604T000000Z,20020605T000000Z,20020606T000000Z,20020607T000000Z,20020608T000000Z,20020609T000000Z,20020610T000000Z,20020611T000000Z,20020612T000000Z,20020613T000000Z,20020614T000000Z,20020615T000000Z,20020616T000000Z,20020617T000000Z,20020618T000000Z,20020619T000000Z,20020620T000000Z,20020621T000000Z,20020622T000000Z,20020623T000000Z,20020624T000000Z,20020625T000000Z,20020626T000000Z,20020627T000000Z,20020628T000000Z,20020629T000000Z,20020630T000000Z,20020701T000000Z,20020702T000000Z,20020703T000000Z,20020704T000000Z,20020705T000000Z,20020706T000000Z,20020707T000000Z,20020708T000000Z,20020709T000000Z,20020710T000000Z,20020711T000000Z,20020712T000000Z,20020713T000000Z,20020714T000000Z,20020715T000000Z,20020716T000000Z,20020717T000000Z,20020718T000000Z,20020719T000000Z,20020720T000000Z,20020721T000000Z,20020722T000000Z,20020723T000000Z,20020724T000000Z,20020725T000000Z,20020726T000000Z,20020727T000000Z,20020728T000000Z,20020729T000000Z,20020730T000000Z,20020731T000000Z,20020801T000000Z,20020802T000000Z,20020803T000000Z,20020804T000000Z,20020805T000000Z,20020806T000000Z,20020807T000000Z,20020808T000000Z,20020809T000000Z,20020810T000000Z,20020811T000000Z,20020812T000000Z,20020813T000000Z,20020814T000000Z,20020815T000000Z,20020816T000000Z,20020817T000000Z,20020818T000000Z,20020819T000000Z,20020820T000000Z,20020821T000000Z,20020822T000000Z,20020823T000000Z,20020824T000000Z,20020825T000000Z,20020826T000000Z,20020827T000000Z,20020828T000000Z,20020829T000000Z,20020830T000000Z,20020831T000000Z,20020901T000000Z,20020902T000000Z,20020903T000000Z,20020904T000000Z,20020905T000000Z,20020906T000000Z,20020907T000000Z,20020908T000000Z,20020909T000000Z,20020910T000000Z,20020911T000000Z,20020912T000000Z,20020913T000000Z,20020914T000000Z,20020915T000000Z,20020916T000000Z,20020917T000000Z,20020918T000000Z,20020919T000000Z,20020920T000000Z,20020921T000000Z,20020922T000000Z,20020923T000000Z,20020924T000000Z,20020925T000000Z,20020926T000000Z"
Max length is 6500, got 17005 (type=value_error)

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • This pull request includes tests or only affects documentation.
  • This pull request includes a label categorizing the change e.g. fix, feature, enhancement

@jakekaplan jakekaplan added DONT MERGE This PR shouldn't be merged (yet) fix A fix for a bug in an existing feature labels Dec 5, 2022
@netlify
Copy link

netlify bot commented Dec 5, 2022 •
edited
Loading

✅ Deploy Preview for prefect-orion ready!

Name Link
🔨 Latest commit 4de6aea
🔍 Latest deploy log https://app.netlify.com/sites/prefect-orion/deploys/63b88611784bf70008618e56
😎 Deploy Preview https://deploy-preview-7762--prefect-orion.netlify.app/api-ref/orion/schemas/schedules
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@jakekaplan jakekaplan requested a review from tpdorsey as a code owner December 5, 2022 16:15
@jakekaplan jakekaplan mentioned this pull request Dec 5, 2022
Closed
8 tasks
@ghost
Copy link

ghost commented Dec 5, 2022

I believe this PR will break our primary mechanism for scheduling flow runs. Let's say we need to run a flow according to the french bond trading calendar, or malaysian bank calendar. We know what dates the market is open, but it's infeasible for us to represent those dates as RRULES. With this change it will be impossible for us to provide a custom list of dates to Prefect for scheduling for more than a year.

If there are no other alternatives for custom date scheduling, I would propose a max length closer to 5 years so that we don't need to refresh the schedule on all of our flows every 12 months.

@ghost
Copy link

ghost commented Dec 5, 2022

Thank you for bumping, we discussed internally and have some ideas about how to shorten the length as well. We believe we can invert the RRULE to be exclusions rather than inclusions and add some special cases for weekends. They will still be quite long, but probably not the full ~6k characters per year.

@jakekaplan
Copy link
Contributor Author

@bphillips-exos that's great to hear! If the type of schedule you've described is able to accomplished via exclusions, I'm honestly inclined to lower it a bit more to something like 12500

@ghost
Copy link

ghost commented Dec 5, 2022

@jakekaplan give us a few days to look into this and get back to you on roughly how many characters per year we need to build the RRULE string for the more complex calendars. Thanks

@ghost
Copy link

ghost commented Dec 12, 2022

Hi @jakekaplan, we were able to switch to EXDATEs for this. Our RRULEs now cap out at ~500 characters per year. At the moment we generate RRULEs for 30 years out so ~15,000 total characters, but we could easily shorten this if you'd prefer to introduce a limit in the 5,000 - 10,000 character range. Thanks for giving us a chance to review

@jakekaplan
Copy link
Contributor Author

@bphillips-exos that's great news! Thanks for looking into it. I'm going to opt to move this back to the original limit of 6500

@jakekaplan jakekaplan removed the DONT MERGE This PR shouldn't be merged (yet) label Dec 12, 2022
@zanieb
Copy link
Contributor

zanieb commented Jan 6, 2023

@jakekaplan can we get this across the line for today's release?

@jakekaplan
Copy link
Contributor Author

@madkinsz updated, sorry that I missed your original comment last week!

zanieb
zanieb reviewed Jan 6, 2023
View reviewed changes
src/prefect/orion/schemas/schedules.py Outdated
@@ -398,7 +398,7 @@ def validate_rrule_str(cls, v):
raise ValueError(f'Invalid RRule string "{v}": {exc}')
if len(v) > MAX_RRULE_LENGTH:
raise ValueError(
f'Invalid RRule string "{v}"\n'
f'Invalid RRule string "{v[:40] + "..." if len(v) > 40 else v}"\n'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we know the length is > 40 since we're in this clause?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was thinking that MAX_RRULE_LENGTH could technically be anything? You're correct though, just simplified it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True but practically speaking 🤷‍♂️ not sure what's best practice in this case.

@zanieb zanieb merged commit ceadcc5 into main Jan 6, 2023
@zanieb zanieb deleted the rrule-limit branch January 6, 2023 21:14
github-actions bot pushed a commit that referenced this pull request Jan 6, 2023
@jakekaplan @madkinsz
Adds MAX_RRULE_LENGTH (#7762)
ffe315a
github-actions bot pushed a commit that referenced this pull request Jan 6, 2023
@jakekaplan @github-actions
Adds MAX_RRULE_LENGTH (#7762)
bcbb1fb
ddelange added a commit to ddelange/prefect that referenced this pull request Jan 9, 2023
@ddelange
Merge branch 'main' of https://github.com/ddelange/prefect into docke…
b8283f3 
…r-builds-release

* 'main' of https://github.com/ddelange/prefect: (131 commits)
  Fix docker-builds.yaml templating syntax (PrefectHQ#8114)
  Rename Worker pools -> work pools (PrefectHQ#8107)
  Build multi-arch images for commits on main (PrefectHQ#7900)
  [Issue PrefectHQ#456] Change example of `infra_override` in docs/concepts/deployment (PrefectHQ#8101)
  Bump @playwright/test from 1.29.1 to 1.29.2 in /orion-ui (PrefectHQ#8105)
  Bump @prefecthq/orion-design from 1.1.53 to 1.1.54 in /orion-ui (PrefectHQ#8104)
  Update deployment docs to include tag and idempotency key (PrefectHQ#7771)
  Add `BaseWorker` and `ProcessWorker` (PrefectHQ#7996)
  Add Peyton and Serina as global code owners (PrefectHQ#8098)
  Add release notes for 2.7.7 (PrefectHQ#8091)
  Add youtube badge (PrefectHQ#8089)
  Adds `MAX_RRULE_LENGTH` (PrefectHQ#7762)
  Limit task run cache key size (PrefectHQ#7275)
  Add --match flag to work queues documentation (PrefectHQ#7768)
  Modify disable ssl setting tests to allow any for headers and timeout (PrefectHQ#8086)
  Add test for allow_failure and quote (PrefectHQ#8055)
  Adds `experimental_field` decorator (PrefectHQ#8066)
  add docs on migrating block documents (PrefectHQ#8085)
  Add Redoc documentation for REST API reference (PrefectHQ#7503)
  Allow disabling SSL verification (PrefectHQ#7850)
  ...
ddelange added a commit to ddelange/prefect that referenced this pull request Jan 9, 2023
@ddelange
Merge branch 'docker-builds-release' of https://github.com/ddelange/p…
e300821 
…refect into docker-builds-consolidation

* 'docker-builds-release' of https://github.com/ddelange/prefect: (131 commits)
  Fix docker-builds.yaml templating syntax (PrefectHQ#8114)
  Rename Worker pools -> work pools (PrefectHQ#8107)
  Build multi-arch images for commits on main (PrefectHQ#7900)
  [Issue PrefectHQ#456] Change example of `infra_override` in docs/concepts/deployment (PrefectHQ#8101)
  Bump @playwright/test from 1.29.1 to 1.29.2 in /orion-ui (PrefectHQ#8105)
  Bump @prefecthq/orion-design from 1.1.53 to 1.1.54 in /orion-ui (PrefectHQ#8104)
  Update deployment docs to include tag and idempotency key (PrefectHQ#7771)
  Add `BaseWorker` and `ProcessWorker` (PrefectHQ#7996)
  Add Peyton and Serina as global code owners (PrefectHQ#8098)
  Add release notes for 2.7.7 (PrefectHQ#8091)
  Add youtube badge (PrefectHQ#8089)
  Adds `MAX_RRULE_LENGTH` (PrefectHQ#7762)
  Limit task run cache key size (PrefectHQ#7275)
  Add --match flag to work queues documentation (PrefectHQ#7768)
  Modify disable ssl setting tests to allow any for headers and timeout (PrefectHQ#8086)
  Add test for allow_failure and quote (PrefectHQ#8055)
  Adds `experimental_field` decorator (PrefectHQ#8066)
  add docs on migrating block documents (PrefectHQ#8085)
  Add Redoc documentation for REST API reference (PrefectHQ#7503)
  Allow disabling SSL verification (PrefectHQ#7850)
  ...
masonmenges pushed a commit that referenced this pull request Jan 10, 2023
@jakekaplan @masonmenges
Adds MAX_RRULE_LENGTH (#7762)
33201e5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zanieb zanieb zanieb approved these changes

@zangell44 zangell44 Awaiting requested review from zangell44

@tpdorsey tpdorsey Awaiting requested review from tpdorsey

@anticorrelator anticorrelator Awaiting requested review from anticorrelator

Assignees
No one assigned
Labels
fix A fix for a bug in an existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jakekaplan @zanieb