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

Add cron validation for cloud.Schedule #5985

Closed
garysassano opened this issue Mar 18, 2024 · 3 comments · Fixed by #6090
Closed

Add cron validation for cloud.Schedule #5985

garysassano opened this issue Mar 18, 2024 · 3 comments · Fixed by #6090
Assignees
@garysassano
Labels
☁️ aws Related to Amazon Web Services support ✨ enhancement New feature or request good first issue Good for newcomers

Comments

@garysassano
Copy link
Collaborator

Use Case

main.w

bring cloud;

let from_cron = new cloud.Schedule( cron: "* * * * ?" ) as "from_cron";`

Running this code should fail immediately, but it works when compiled to AWS platforms.

Proposed Solution

We should add a check that the cron string is a valid unix-cron.

Implementation Notes

Use cron-validate or a similar library.

Component

SDK

Community Notes

  • Please vote by adding a 👍 reaction to the issue to help us prioritize.
  • If you are interested to work on this issue, please leave a comment.
  • If this issue is labeled needs-discussion, it means the spec has not been finalized yet. Please reach out on the #dev channel in the Wing Slack.
@garysassano garysassano added ✨ enhancement New feature or request ☁️ aws Related to Amazon Web Services support needs-discussion Further discussion is needed prior to impl labels Mar 18, 2024
@staycoolcall911 staycoolcall911 added the good first issue Good for newcomers label Mar 20, 2024
@Chriscbr Chriscbr removed the needs-discussion Further discussion is needed prior to impl label Mar 20, 2024
@marciocadev
Copy link
Collaborator

marciocadev commented Mar 25, 2024 •
edited

@garysassano

That was fixed in fix(sdk): making cron schedule more cloud agnostic (second attempt)

I think we can close this issue.

@garysassano
Copy link
Collaborator Author

@marciocadev You can see the problem here. If everything was working correctly, this test would fail, because that's not a valid unix-cron. That needs to be fixed and an extra test added to check that the passed cron is actually valid.

@garysassano garysassano mentioned this issue Mar 29, 2024
Merged
5 tasks
@garysassano garysassano self-assigned this Mar 29, 2024
@mergify mergify bot closed this as completed in #6090 Mar 29, 2024
mergify bot pushed a commit that referenced this issue Mar 29, 2024
@garysassano
fix(sdk): add cron validation for cloud.Schedule (#6090)
9eb53bb 
Closes #5985

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.66.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@garysassano garysassano

Labels
☁️ aws Related to Amazon Web Services support ✨ enhancement New feature or request good first issue Good for newcomers
Projects
Wing
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(sdk): add cron validation for cloud.Schedule winglang/wing
5 participants
@Chriscbr @garysassano @marciocadev @staycoolcall911 @monadabot