- Notifications
You must be signed in to change notification settings - Fork928
feat: notify owner about failed autobuild#13891
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
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.
Overall LGTM! A few relatively minor points.
coderd/database/migrations/000226_notifications_autobuild_failed.up.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.
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.
| ignoreLogErrors := true | ||
| srv, db, ps, pd := setup(t, ignoreLogErrors, &overrides{ | ||
| notificationEnqueuer: notifEnq, | ||
| }) | ||
| user := dbgen.User(t, db, database.User{}) | ||
| initiator := user | ||
| template := dbgen.Template(t, db, database.Template{ | ||
| Name: "template", | ||
| Provisioner: database.ProvisionerTypeEcho, | ||
| OrganizationID: pd.OrganizationID, | ||
| }) | ||
| template, err := db.GetTemplateByID(ctx, template.ID) | ||
| require.NoError(t, err) | ||
| file := dbgen.File(t, db, database.File{CreatedBy: user.ID}) | ||
| workspace := dbgen.Workspace(t, db, database.Workspace{ | ||
| TemplateID: template.ID, | ||
| OwnerID: user.ID, | ||
| OrganizationID: pd.OrganizationID, | ||
| }) | ||
| version := dbgen.TemplateVersion(t, db, database.TemplateVersion{ | ||
| OrganizationID: pd.OrganizationID, | ||
| TemplateID: uuid.NullUUID{ | ||
| UUID: template.ID, | ||
| Valid: true, | ||
| }, | ||
| JobID: uuid.New(), | ||
| }) | ||
| build := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ | ||
| WorkspaceID: workspace.ID, | ||
| TemplateVersionID: version.ID, | ||
| InitiatorID: initiator.ID, | ||
| Transition: database.WorkspaceTransitionDelete, | ||
| Reason: tc.buildReason, | ||
| }) | ||
| job := dbgen.ProvisionerJob(t, db, ps, database.ProvisionerJob{ | ||
| FileID: file.ID, | ||
| Type: database.ProvisionerJobTypeWorkspaceBuild, | ||
| Input: must(json.Marshal(provisionerdserver.WorkspaceProvisionJob{ | ||
| WorkspaceBuildID: build.ID, | ||
| })), | ||
| OrganizationID: pd.OrganizationID, | ||
| }) | ||
| _, err = db.AcquireProvisionerJob(ctx, database.AcquireProvisionerJobParams{ | ||
| OrganizationID: pd.OrganizationID, | ||
| WorkerID: uuid.NullUUID{ | ||
| UUID: pd.ID, | ||
| Valid: true, | ||
| }, | ||
| Types: []database.ProvisionerType{database.ProvisionerTypeEcho}, | ||
| }) | ||
| require.NoError(t, err) |
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: all the common bits here could be moved out to a helper func.
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.
Yeah, my problem with this idea is that we already havesetup() function, so we will add another, let's sayprepare(), butprepare() would be used only by 2/10 tests in the unit.
I think I will pass on it.
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.
LGTM, some missing/confusing aspects of the tests that I think we need to fix but after that this is ready to go.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
a5e4bf3 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Related:coder/internal#7
This PR implements a new notification: workspace autobuild failure.
Changes:
Workspace Autobuild FailedinitiatedBywithinitiatorinitiatorwhen autobuild deletes a workspace