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

Modifying schedules API to allow for rrulesets #5733 #12043

Merged
merged 7 commits into from
Apr 28, 2022

Conversation

john-westcott-iv
Copy link
Member

@john-westcott-iv john-westcott-iv commented Apr 13, 2022

SUMMARY

Modifies the code to allow for more complex rrulesets instead of just a single RRULE.
You can add multiple RRULEs in addition to EXRULES (exclude rules).
However, this does not include support for DATES and EXDATES.

NOTE: There is one piece (the until property) that will need additional changes in the future when the UI also updates. For now, we left the until computation alone which returns the first until found in the ruleset. Once the UI work starts we will need to determine if this returns an array of untils or if this property goes away and the until's will be computed on the client side while processing the ruleset.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • API
AWX VERSION
awx: 20.0.2.dev222+gdd045bf120
ADDITIONAL INFORMATION

@nixocio
Copy link
Contributor

nixocio commented Apr 18, 2022

@john-westcott-iv there is a JS lint failure. From the ui dir, npm run prettier && npm run lint.

Copy link
Contributor

@nixocio nixocio left a comment

Choose a reason for hiding this comment

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

I verified the UI changes. Great job @john-westcott-iv.

@AlanCoding
Copy link
Member

I would like to see some docs with this to get the ball rolling. docs/schedules.md seems okay for starting that, because this stuff about rrule is fairly contained, and that has the last-best stuff.

awx/api/serializers.py Outdated Show resolved Hide resolved
* The use of `FREQ=BYWEEKNO` in an `RRULE`
* The use of `COUNT=` in an `RRULE` with a value over 999

* At least one RRULE must be in the string
Copy link
Member

Choose a reason for hiding this comment

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

A little confused - isn't this a list of things that are not supported? This looks like a true statement.

Copy link
Member Author

Choose a reason for hiding this comment

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

The wording here has been updated.


DTSTART:20191219T130551Z RRULE:FREQ=MONTHLY;INTERVAL=1;BYDAY=SA;BYMONTHDAY=12,13,14,15,16,17,18


Copy link
Member

Choose a reason for hiding this comment

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

👍 great example content, thanks for adding those.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll be adding these to the unit tests as well.

@AlanCoding
Copy link
Member

AlanCoding commented Apr 19, 2022

@nixocio what's the state of this issue?

There is one piece (the until property) that will need additional changes in the future when the UI also updates.

With the UI changes you added, is this now a problem? (I read it that you can set "until" constraints in the UI that the API will choke on)

EDIT: never mind, I realize there isn't really UI work done yet.

Changing rrule validation to allow for multuipe BYMONTH, BYMONTHDAY, BYYEARDAY and BYYEARDAY

Modified validation to inspect for SECONDLY at the rule level and now returning a list of all errors instead of the first encounterd

Fixed some linting in the rruleset test modules
@cypress
Copy link

cypress bot commented Apr 25, 2022



Test summary

638 0 821 0Flakiness 1


Run details

Project AWX - Functional
Status Passed
Commit 065e7e6
Started Apr 25, 2022 4:55 PM
Ended Apr 25, 2022 6:44 PM
Duration 49:56 💡
OS Linux Debian - 11.3
Browser Chrome 99

View run in Cypress Dashboard ➡️


Flakiness

cypress/integration/hosts/hosts-lists.spec.js Flakiness
1 Host list > can sort hosts by name

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@john-westcott-iv john-westcott-iv merged commit c67f508 into ansible:devel Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants