Movatterモバイル変換

Skip to content

coder/coderPublic

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

Add`reason` field for workspace builds#2438

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

AbhineetJain merged 12 commits intomainfromabhineetjain/2029-show-build-reason-in-ui

Jun 17, 2022

Merged

Add`reason` field for workspace builds#2438

AbhineetJain merged 12 commits intomainfromabhineetjain/2029-show-build-reason-in-ui

Jun 17, 2022

Conversation

AbhineetJain

Copy link

Contributor

AbhineetJain commentedJun 16, 2022•
edited
Loading

This PR adds a newreason field for workspace builds to indicate the reason for its initiation. The following values are supported:initiator,autostart,autostop.

The following combinations can arise:

Initiator	Reason
initiator_id	Reason.Initiator
owner_id	Reason.Autostart
owner_id	Reason.Autostop

In case ofReason.Autostart andReason.Autostop, we ignore the initiator field when displaying it in the UI, and use the initiator field for the other case - the member could be admin too (or whoever the user may possibly be sharing workspaces with).

Subtasks

added a migration to add the new column
used the new column while inserting the values into workspace builds table
update databasefake

kylecarbs

kylecarbs approved these changes

View reviewed changes

Copy link

Member

kylecarbs left a comment

Choose a reason for hiding this comment

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

I really like this approach! Nice n' simple 😎

coderd/database/models.go OutdatedShow resolvedHide resolved

@AbhineetJain

AbhineetJain requested a review froma team as acode owner

June 16, 2022 20:59

@AbhineetJain

AbhineetJain mentioned this pull request

Add system user with special uuid#2428

Closed

5 tasks

Emyrk

Emyrk approved these changes

View reviewed changes

Copy link

Member

Emyrk left a comment

Choose a reason for hiding this comment

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

LG. Minor nit

codersdk/workspacebuilds.go OutdatedShow resolvedHide resolved

@AbhineetJain

Copy link

ContributorAuthor

AbhineetJain commentedJun 16, 2022

Handles the backend changes required for issues:#2029,#2410

This was referencedJun 16, 2022

Show build initiator on Workspace Build page#2446

Merged

Show the reason for workspace action on build page#2410

Closed

johnstcn

johnstcn reviewed

View reviewed changes

Copy link

Member

johnstcn left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me! I Just a couple small issues.

coderd/autobuild/executor/lifecycle_executor.go Outdated

		Transition: trans,
		JobID: newProvisionerJob.ID,

		err = store.InTx(func(db database.Store) error {

Copy link

Member

johnstcnJun 17, 2022

Choose a reason for hiding this comment

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

This is already in one big transaction, see line 80 -- since we're already in a tx, this ends up being a no-op

Copy link

Member

EmyrkJun 17, 2022

Choose a reason for hiding this comment

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

Oh TIL. I didn't see that@AbhineetJain

Copy link

ContributorAuthor

AbhineetJainJun 17, 2022•
edited
Loading

Choose a reason for hiding this comment

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

Thanks@johnstcn. This was also the reason the tests were failing.

Copy link

ContributorAuthor

AbhineetJainJun 17, 2022

Choose a reason for hiding this comment

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

Issue:#2468

coderd/autobuild/executor/lifecycle_executor_test.go OutdatedShow resolvedHide resolved

coderd/autobuild/executor/lifecycle_executor_test.goShow resolvedHide resolved

AbhineetJain added11 commits

June 17, 2022 16:13

@AbhineetJain

add reason field for workspace build

ee90841

@AbhineetJain

add the reason field to FE via API

d460617

@AbhineetJain

update BuildReasonMember to BuildReasonInitiator

105ccfa

@AbhineetJain

add unit tests

90e0d17

@AbhineetJain

add more unit tests

d103f69

@AbhineetJain

add error for unknown transition

fa38b8f

@AbhineetJain

fix lint

2e8c462

@AbhineetJain

add documentation

6fb655f

@AbhineetJain

fix unit tests

eacd6a0

@AbhineetJain

fix generated types

b7288b1

@AbhineetJain

remove nested transaction

ae7e908

@AbhineetJain

AbhineetJain force-pushed theabhineetjain/2029-show-build-reason-in-ui branch from778b819 to4802eafCompare

June 17, 2022 16:15

@AbhineetJain

rename migration file

c5b9aac

@AbhineetJain

AbhineetJain force-pushed theabhineetjain/2029-show-build-reason-in-ui branch from4802eaf toc5b9aacCompare

June 17, 2022 16:21

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.

Nice work!

Kira-Pilot

Kira-Pilot approved these changes

View reviewed changes

@AbhineetJain

AbhineetJain merged commit289b989 intomain

@AbhineetJain

AbhineetJain deleted the abhineetjain/2029-show-build-reason-in-ui branch

June 17, 2022 17:41

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

Labels

None yet

5 participants

@AbhineetJain

@johnstcn

@Emyrk

@kylecarbs

@Kira-Pilot

[8]ページ先頭

©2009-2025 Movatter.jp