Movatterモバイル変換

Skip to content

coder/coderPublic

NotificationsYou must be signed in to change notification settings
Fork927
Star10.1k

feat: notify about manual failed builds#14419

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.

Sign up for GitHub

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

Merged

mtojek merged 16 commits intomainfrom16-manual-build-failed

Aug 27, 2024

Merged

feat: notify about manual failed builds#14419

mtojek merged 16 commits intomainfrom16-manual-build-failed

Aug 27, 2024

Conversation

mtojek

Copy link

Member

mtojek commentedAug 23, 2024•
edited
Loading

Fixes:coder/internal#16

This PR modifies ProvisionerdServer to notify template admins about manual workspace build failures.

@mtojek

Manual build failed

24e8c1b

@mtojek

mtojek self-assigned this

mtojek added5 commits

August 23, 2024 15:58

@mtojek

fix

0782614

@mtojek

WIP

2342d52

@mtojek

simple test

127aa47

@mtojek

fix

e9737eb

@mtojek

Merge branch 'main' into 16-manual-build-failed

a13eb9e

@mtojek

mtojek mentioned this pull request

feat: notify template owner on manual build failures#14262

Closed

@mtojek

fix

0d889b5

@mtojek

mtojek requested review fromdannykopping andBrunoQuaresma

August 26, 2024 13:14

@mtojek

mtojek marked this pull request as ready for review

August 26, 2024 13:14

BrunoQuaresma

BrunoQuaresma reviewed

View reviewed changes

coderd/database/migrations/000248_notifications_manual_build_failed.up.sql OutdatedShow resolvedHide resolved

mtojek added2 commits

August 26, 2024 15:22

@mtojek

double n

015ec3c

@mtojek

expose template version

6a0b9f7

johnstcn

johnstcn reviewed

View reviewed changes

coderd/provisionerdserver/provisionerdserver.go OutdatedShow resolvedHide resolved

coderd/provisionerdserver/provisionerdserver_test.go

Comment on lines +1851 to +1861

		assert.Equal(t, notifEnq.Sent[0].UserID, templateAdmin.ID)
		assert.Equal(t, notifEnq.Sent[0].TemplateID, notifications.TemplateWorkspaceManualBuildFailed)
		assert.Contains(t, notifEnq.Sent[0].Targets, template.ID)
		assert.Contains(t, notifEnq.Sent[0].Targets, workspace.ID)
		assert.Contains(t, notifEnq.Sent[0].Targets, workspace.OrganizationID)
		assert.Contains(t, notifEnq.Sent[0].Targets, user.ID)
		assert.Equal(t, workspace.Name, notifEnq.Sent[0].Labels["name"])
		assert.Equal(t, template.Name, notifEnq.Sent[0].Labels["template_name"])
		assert.Equal(t, version.Name, notifEnq.Sent[0].Labels["template_version_name"])
		assert.Equal(t, user.Username, notifEnq.Sent[0].Labels["initiator"])
		assert.Equal(t, user.Username, notifEnq.Sent[0].Labels["workspace_owner_username"])

Copy link

Member

johnstcnAug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I feel like these assertions could be passed into the previous table-based test "Workspace build failed"?

Copy link

MemberAuthor

mtojekAug 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

There is a significant difference as here we expect notifications sent only to template admins, we don't care about workspace owners. I tried to squeeze this unit test as much as possible.

coderd/database/migrations/000248_notifications_manual_build_failed.up.sql OutdatedShow resolvedHide resolved

dannykopping

dannykopping reviewed

View reviewed changes

coderd/provisionerdserver/provisionerdserver.goShow resolvedHide resolved

coderd/provisionerdserver/provisionerdserver.go OutdatedShow resolvedHide resolved

coderd/provisionerdserver/provisionerdserver.go OutdatedShow resolvedHide resolved

mtojek added5 commits

August 27, 2024 10:37

@mtojek

fix: logging

f03cbc0

@mtojek

build number

30f3809

@mtojek

prepare

c2f038c

@mtojek

fmt

2381213

@mtojek

fmt

51b880c

dannykopping

dannykopping approved these changes

View reviewed changes

coderd/provisionerdserver/provisionerdserver.go

		@@ -1150,15 +1130,43 @@ func (s *server) notifyWorkspaceManualBuildFailed(ctx context.Context, workspace
		"template_version_name": templateVersion.Name,
		"initiator": build.InitiatorByUsername,
		"workspace_owner_username": workspaceOwner.Username,
		"workspace_build_number": strconv.Itoa(int(build.BuildNumber)),

Copy link

Contributor

dannykoppingAug 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Including this will mean that deduplication won't be possible, just so you're aware.

coderd/provisionerdserver/provisionerdserver.goShow resolvedHide resolved

@mtojek

test

e4f6f29

@mtojek

mtojek requested a review fromjohnstcn

August 27, 2024 13:24

johnstcn

johnstcn approved these changes

View reviewed changes

Copy link

Member

johnstcn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍

coderd/provisionerdserver/provisionerdserver.goShow resolvedHide resolved

@mtojek

comment

53f6eab

@mtojek

mtojek mentioned this pull request

feat: template notificationscoder/internal#16

Closed

2 tasks

@mtojek

mtojekenabled auto-merge (squash)

August 27, 2024 14:27

@mtojek

mtojek merged commit47f2c7d intomain

30 checks passed

@mtojek

mtojek deleted the 16-manual-build-failed branch

August 27, 2024 14:35

@github-actions

github-actionsbot locked and limited conversation to collaborators

Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Labels

None yet

4 participants

@mtojek

@dannykopping

@BrunoQuaresma

@johnstcn

[8]ページ先頭

©2009-2025 Movatter.jp