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

CronWorkflow schedule giving inconsistent experience #5691

Closed
dinever opened this issue Apr 17, 2021 · 1 comment · Fixed by #5692
Closed

CronWorkflow schedule giving inconsistent experience #5691

dinever opened this issue Apr 17, 2021 · 1 comment · Fixed by #5692
Assignees
Labels
Milestone

Comments

@dinever
Copy link
Member

dinever commented Apr 17, 2021

Summary

This issue contains several minor issues related to CronWorkflow:

AFIAK we don't support the second-level CronWorkflow schedule. Second-level CronWorkflow will be rejected when submitting. But the crontab translator seems to have no problem with it. This is an inconsistent user experience:

Screen Shot 2021-04-17 at 1 00 58 AM

Furthermore, for an existing & valid CronWorkflow, the user can actually update it to a second-level CronWorkflow, and the schedule doesn't seem to work (Next schedule time says "xx seconds ago").

crontab

We should have consistent UX for this.


Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

@alexec
Copy link
Contributor

alexec commented Apr 17, 2021

Changes needed:

  1. Fix UpdateCronWorkflow (server/cronworkflow/cron_workflow_server.go) to invoke validate.ValidateCronWorkflow (like CreateCronWorkflow` does on line 35)
  2. Remove Next Scheduled Time (ui/src/app/cron-workflows/components/cron-workflow-status-viewer.tsx#L27-35). I think it is better to remove this feature than it to be buggy because it is based on wrong information.
  3. The UI validator resides in <ScheduleValidator/> (ui/src/app/cron-workflows/components/schedule-validator.tsx). However, this will always only be roughly correct - and we cannot predict when it might be wrong. Instead, we should add an <InfoIcon /> to let users know that seconds are not supported.

@alexec alexec added this to the v3.0 milestone Apr 17, 2021
@alexec alexec added the P2 label Apr 17, 2021
alexec added a commit to alexec/argo-workflows that referenced this issue Apr 17, 2021
@alexec alexec linked a pull request Apr 17, 2021 that will close this issue
@alexec alexec self-assigned this Apr 17, 2021
@simster7 simster7 mentioned this issue Apr 19, 2021
50 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants