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

fix(ui): ignore @every cron expression for FE pretty printing and update cronstrue dependency. Fixes #13489 #13586

Merged
merged 3 commits into from
Oct 3, 2024

Conversation

djanjic
Copy link
Contributor

@djanjic djanjic commented Sep 11, 2024

Fixes #13489

Motivation

This change should address confusion when submitting/ checking a cron workflow using UI.

Modifications

  • Updated pretty-schedule.tsx to ignore cron expressions in @every format since library used for pretty printing is not supporting this format. schedule-validation.tsx is not updated with this change but I'm still considering that since there's a BE validation. For @every format, there's no pretty printed description nor next schedule date.
  • @daily, @hourly and other formats supported by BE library (https://github.com/robfig/cron) are support in FE js library in the latest version (Support for '@' expressions bradymholt/cRonstrue#318). This PR contains dependency update to include this change.

Verification

Check screenshots:
Screenshot 2024-09-11 at 13 52 54
Screenshot 2024-09-11 at 13 53 14

@djanjic djanjic changed the title fix(ui): ignore @every cron expression for FE validation and update c… fix(ui): ignore @every cron expression for FE validation and update cronstrue dependency. Fixes #13489 Sep 11, 2024
@djanjic djanjic changed the title fix(ui): ignore @every cron expression for FE validation and update cronstrue dependency. Fixes #13489 fix(ui): ignore @every cron expression for FE pretty printing and update cronstrue dependency. Fixes #13489 Sep 11, 2024
@djanjic djanjic marked this pull request as ready for review September 11, 2024 13:24
@agilgur5 agilgur5 changed the title fix(ui): ignore @every cron expression for FE pretty printing and update cronstrue dependency. Fixes #13489 fix(ui): ignore @every cron expression for FE pretty printing and update cronstrue dependency. Fixes #13489 Sep 11, 2024
@agilgur5
Copy link
Contributor

Fixes #13489

Oh, we thought this was an API validation bug, but I guess not, it's only the UI that's affected?

@Joibel
Copy link
Member

Joibel commented Sep 11, 2024

Oh, we thought this was an API validation bug, but I guess not, it's only the UI that's affected?

Indeed, I did as well.

Had me a bit foxed looking at the validator and not seeing where it was going wrong.

@agilgur5
Copy link
Contributor

agilgur5 commented Sep 11, 2024

  • Updated pretty-schedule.tsx to ignore cron expressions in @every format since library used for pretty printing is not supporting this format.

Have you considered contributing @every support upstream?

@agilgur5
Copy link
Contributor

Have you considered contributing @every support upstream?

Nevermind, apparently @every is a non-standard extension from robfig/cron and uses the Go specific time.Duration format that k8s API conventions recommend against

@agilgur5 agilgur5 added type/dependencies PRs and issues specific to updating dependencies area/ui javascript Pull requests that update Javascript dependencies area/cron-workflows labels Sep 11, 2024
@Joibel Joibel merged commit ba75efb into argoproj:main Oct 3, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cron-workflows area/ui javascript Pull requests that update Javascript dependencies type/dependencies PRs and issues specific to updating dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI: Allow @schedules in CronWorkflow field
3 participants