- Notifications
You must be signed in to change notification settings - Fork928
feat: add crontab package for supporting autostart/stop.#844
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This is basically a small wrapper around robfig/cron/v3.Fixes#817.
codecovbot commentedApr 4, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## main #844 +/- ##==========================================+ Coverage 65.72% 65.85% +0.12%========================================== Files 202 203 +1 Lines 13209 13227 +18 Branches 87 87 ==========================================+ Hits 8681 8710 +29+ Misses 3637 3626 -11 Partials 891 891
Continue to review full report at Codecov.
|
…is basically a small wrapper around robfig/cron/v3.
…is basically a small wrapper around robfig/cron/v3.
…. This is basically a small wrapper around robfig/cron/v3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm nervous about generalizing a CRON package with the single use-case in mind. It's not that this is the wrong abstraction, but it's guesswork on my part to tell if it's the right one since it's not plugged into anything.
You likely have a clearer picture of how this will play out than I do, so I shall approve regardless. It looks well tested and structurally aligned, but it's hard to truly know until we use it!
coderd/crontab/crontab.go Outdated
| // WeeklySchedule represents a weekly cron schedule. | ||
| // It's essentially a thin wrapper for robfig/cron/v3 that implements Stringer. | ||
| type WeeklySchedule struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This weekly schedule feels pretty specified to autostart/stop. Could we add this to the autostart/stop package once that's in (assuming it'll be inside it's own package)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Also, I'm not sure this needs to haveWeekly as a prefix. That seems like part of the parsing logic, not the struct functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I can move this tocoderd/autostart/schedule if that works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Based on what's here, I'd say that makes sense. I'm uncertain what structurally makes sense until we have more of the implementation worked out, but as long as we're not opposed to moving this afterwards, I'm good with it!
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Summary:This adds the client-side implementation to match the types introducedin#879 and#844 as well as a card in the Workspaces page to presentworkspace the data.Details:* Added a convenient line break in the example schedule.Weekly* Added missing `json:""` annotations in codersdk/workspaces.go* Installed cronstrue for displaying human-friendly cron strings* Adjusted/Added client-side types to match codersdk/workspaces.go* Added new component WorkspaceSchedule.tsxNext Steps:The WorkspaceSchedule.tsx card only presents data (on purpose). In orderto make it PUT/modify data, a few changes will be made:- a form for updating workspace schedule will be created- the form will wrapped in a dialog or modal- the WorkspaceSchedule card will have a way of opening the modal whichwill likely be generalized up to WorkspaceSection.tsxImpact:This is user-facingThis does not fully resolve either#274 or#275 (I may further decomposethat work to reflect reality and keep things in small deliverableincrements), but adds significant progress towards both.
Summary:This adds the client-side implementation to match the types introducedin#879 and#844 as well as a card in the Workspaces page to presentworkspace the data.Details:* Added a convenient line break in the example schedule.Weekly* Added missing `json:""` annotations in codersdk/workspaces.go* Installed cronstrue for displaying human-friendly cron strings* Adjusted/Added client-side types to match codersdk/workspaces.go* Added new component WorkspaceSchedule.tsxNext Steps:The WorkspaceSchedule.tsx card only presents data (on purpose). In orderto make it PUT/modify data, a few changes will be made:- a form for updating workspace schedule will be created- the form will wrapped in a dialog or modal- the WorkspaceSchedule card will have a way of opening the modal whichwill likely be generalized up to WorkspaceSection.tsxImpact:This is user-facingThis does not fully resolve either#274 or#275 (I may further decomposethat work to reflect reality and keep things in small deliverableincrements), but adds significant progress towards both.
This is basically a small wrapper around robfig/cron/v3.
Fixes#817.