- Notifications
You must be signed in to change notification settings - Fork928
coderd: autostart: codersdk, http api, database plumbing#879
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
codecovbot commentedApr 5, 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 #879 +/- ##==========================================+ Coverage 65.77% 65.98% +0.21%========================================== Files 216 216 Lines 13734 13834 +100 Branches 103 103 ==========================================+ Hits 9033 9129 +96 Misses 3782 3782- Partials 919 923 +4
Continue to review full report at Codecov.
|
coderd/database/migrations/000005_workspace_autostartstop.down.sql OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
kylecarbs left a comment• 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.
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.
Due to the to-dos I'm going to leave in commented state, but all looks great so far! Just oneextremely minor nit.
coderd/workspaces_test.go Outdated
| ) | ||
| err := client.UpdateWorkspaceAutostart(ctx, wsid, req) | ||
| require.EqualError(t, err, fmt.Sprintf("status code 404: workspace %q does not exist", wsid), "unexpected error") |
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.
nit: We could assert tocodersdk.Error here to check the status code!
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.
ack
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.
FYI I ended up needing to squelch a linter error when trying to useerrors.As oncodersdk.Error.
This is the sort of thing I ended up with:https://go.dev/play/p/NHTmnYWan6g
- add unit tests to and from DST- ensure expected interval between autostarts
…/ UpdateWorkspaceAutostart
a17d3a1 to316501dCompareSummary: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.
Uh oh!
There was an error while loading.Please reload this page.
putWorkspaceAutostartUpdateWorkspaceAutostartmethod🌮 to@vapurrmaid on the help with TDD!
NOTE: I also added the autostop-related database stuff before it was deemed better to submit smaller more integrated PRs. If this is a blocker, I can remove it from this PR and add it in the next. But it will likely be used Very Soon.
ALSO NOTE: I'm not using "fixes" here because the issues I opened are "horizontally" aligned, and not "vertically".