Skip to content

cleanup workspace machine #4160

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 23, 2022
Merged

cleanup workspace machine #4160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7f3cd90
removed dead build states
Kira-Pilot Sep 21, 2022
8740c82
removed dead code
Kira-Pilot Sep 21, 2022
0cd5889
removed guards
Kira-Pilot Sep 21, 2022
463708b
not calling events from actions
Kira-Pilot Sep 22, 2022
e206231
Merge remote-tracking branch 'origin/main' into cleanup-workspace-mac…
Kira-Pilot Sep 22, 2022
ee657f2
simplified timeline
Kira-Pilot Sep 23, 2022
d515a69
simplify refresh template
Kira-Pilot Sep 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions site/src/pages/WorkspacePage/WorkspacePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const WorkspacePage: FC = () => {
workspace,
getWorkspaceError,
template,
refreshTemplateWarning,
getTemplateWarning,
refreshWorkspaceWarning,
builds,
getBuildsError,
Expand Down Expand Up @@ -72,7 +72,7 @@ export const WorkspacePage: FC = () => {
return (
<div className={styles.error}>
{Boolean(getWorkspaceError) && <ErrorSummary error={getWorkspaceError} />}
{Boolean(refreshTemplateWarning) && <ErrorSummary error={refreshTemplateWarning} />}
{Boolean(getTemplateWarning) && <ErrorSummary error={getTemplateWarning} />}
{Boolean(checkPermissionsError) && <ErrorSummary error={checkPermissionsError} />}
</div>
)
Expand Down
160 changes: 76 additions & 84 deletions site/src/xServices/workspace/workspaceXService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { assign, createMachine, send } from "xstate"
import { pure } from "xstate/lib/actions"
import * as API from "../../api/api"
import * as Types from "../../api/types"
import * as TypesGen from "../../api/typesGenerated"
Expand All @@ -12,8 +11,31 @@ const latestBuild = (builds: TypesGen.WorkspaceBuild[]) => {
})[0]
}

const moreBuildsAvailable = (
context: WorkspaceContext,
event: {
type: "REFRESH_TIMELINE"
checkRefresh?: boolean
data?: TypesGen.ServerSentEvent["data"]
},
) => {
// No need to refresh the timeline if it is not loaded
if (!context.builds) {
return false
}

if (!event.checkRefresh) {
return true
}

// After we refresh a workspace, we want to check if the latest
// build was updated before refreshing the timeline so as to not over fetch the builds
const latestBuildInTimeline = latestBuild(context.builds)
return event.data.latest_build.updated_at !== latestBuildInTimeline.updated_at
}

const Language = {
refreshTemplateWarning: "Error updating workspace: latest template could not be fetched.",
getTemplateWarning: "Error updating workspace: latest template could not be fetched.",
buildError: "Workspace action failed.",
}

Expand All @@ -28,7 +50,7 @@ export interface WorkspaceContext {
getWorkspaceError?: Error | unknown
// these are labeled as warnings because they don't make the page unusable
refreshWorkspaceWarning?: Error | unknown
refreshTemplateWarning: Error | unknown
getTemplateWarning: Error | unknown
// Builds
builds?: TypesGen.WorkspaceBuild[]
getBuildsError?: Error | unknown
Expand All @@ -52,8 +74,7 @@ export type WorkspaceEvent =
| { type: "CANCEL_DELETE" }
| { type: "UPDATE" }
| { type: "CANCEL" }
| { type: "CHECK_REFRESH_TIMELINE"; data: TypesGen.ServerSentEvent["data"] }
| { type: "REFRESH_TIMELINE" }
| { type: "REFRESH_TIMELINE"; checkRefresh?: boolean; data?: TypesGen.ServerSentEvent["data"] }
| { type: "EVENT_SOURCE_ERROR"; error: Error | unknown }

export const checks = {
Expand Down Expand Up @@ -140,7 +161,7 @@ export const workspaceMachine = createMachine(
onDone: [
{
actions: "assignWorkspace",
target: "refreshingTemplate",
target: "gettingTemplate",
},
],
onError: [
Expand All @@ -152,11 +173,11 @@ export const workspaceMachine = createMachine(
},
tags: "loading",
},
refreshingTemplate: {
entry: "clearRefreshTemplateWarning",
gettingTemplate: {
entry: "clearGettingTemplateWarning",
invoke: {
src: "getTemplate",
id: "refreshTemplate",
id: "getTemplate",
onDone: [
{
actions: "assignTemplate",
Expand All @@ -165,7 +186,7 @@ export const workspaceMachine = createMachine(
],
onError: [
{
actions: ["assignRefreshTemplateWarning", "displayRefreshTemplateWarning"],
actions: ["assignGetTemplateWarning", "displayGetTemplateWarning"],
target: "error",
},
],
Expand Down Expand Up @@ -211,9 +232,6 @@ export const workspaceMachine = createMachine(
EVENT_SOURCE_ERROR: {
target: "error",
},
CHECK_REFRESH_TIMELINE: {
actions: ["refreshTimeline"],
},
},
},
error: {
Expand Down Expand Up @@ -255,7 +273,7 @@ export const workspaceMachine = createMachine(
src: "startWorkspaceWithLatestTemplate",
onDone: {
target: "idle",
actions: ["assignBuild", "refreshTimeline"],
actions: ["assignBuild"],
},
onError: {
target: "idle",
Expand All @@ -270,7 +288,7 @@ export const workspaceMachine = createMachine(
id: "startWorkspace",
onDone: [
{
actions: ["assignBuild", "refreshTimeline"],
actions: ["assignBuild"],
target: "idle",
},
],
Expand All @@ -289,7 +307,7 @@ export const workspaceMachine = createMachine(
id: "stopWorkspace",
onDone: [
{
actions: ["assignBuild", "refreshTimeline"],
actions: ["assignBuild"],
target: "idle",
},
],
Expand All @@ -308,7 +326,7 @@ export const workspaceMachine = createMachine(
id: "deleteWorkspace",
onDone: [
{
actions: ["assignBuild", "refreshTimeline"],
actions: ["assignBuild"],
target: "idle",
},
],
Expand All @@ -327,11 +345,7 @@ export const workspaceMachine = createMachine(
id: "cancelWorkspace",
onDone: [
{
actions: [
"assignCancellationMessage",
"displayCancellationMessage",
"refreshTimeline",
],
actions: ["assignCancellationMessage", "displayCancellationMessage"],
target: "idle",
},
],
Expand All @@ -343,31 +357,11 @@ export const workspaceMachine = createMachine(
],
},
},
refreshingTemplate: {
entry: "clearRefreshTemplateWarning",
invoke: {
src: "getTemplate",
id: "refreshTemplate",
onDone: [
{
actions: "assignTemplate",
target: "requestingStart",
},
],
onError: [
{
actions: ["assignRefreshTemplateWarning", "displayRefreshTemplateWarning"],
target: "idle",
},
],
},
},
},
},
timeline: {
initial: "gettingBuilds",
states: {
idle: {},
gettingBuilds: {
entry: "clearGetBuildsError",
invoke: {
Expand All @@ -381,19 +375,17 @@ export const workspaceMachine = createMachine(
onError: [
{
actions: "assignGetBuildsError",
target: "idle",
target: "loadedBuilds",
},
],
},
},
loadedBuilds: {
initial: "idle",
states: {
idle: {
on: {
REFRESH_TIMELINE: {
target: "#workspaceState.ready.timeline.gettingBuilds",
},
on: {
REFRESH_TIMELINE: {
target: "#workspaceState.ready.timeline.gettingBuilds",
cond: {
type: "moreBuildsAvailable",
},
},
},
Expand Down Expand Up @@ -484,14 +476,14 @@ export const workspaceMachine = createMachine(
clearRefreshWorkspaceWarning: assign({
refreshWorkspaceWarning: (_) => undefined,
}),
assignRefreshTemplateWarning: assign({
refreshTemplateWarning: (_, event) => event.data,
assignGetTemplateWarning: assign({
getTemplateWarning: (_, event) => event.data,
}),
displayRefreshTemplateWarning: () => {
displayError(Language.refreshTemplateWarning)
displayGetTemplateWarning: () => {
displayError(Language.getTemplateWarning)
},
clearRefreshTemplateWarning: assign({
refreshTemplateWarning: (_) => undefined,
clearGettingTemplateWarning: assign({
getTemplateWarning: (_) => undefined,
}),
// Timeline
assignBuilds: assign({
Expand All @@ -503,24 +495,9 @@ export const workspaceMachine = createMachine(
clearGetBuildsError: assign({
getBuildsError: (_) => undefined,
}),
refreshTimeline: pure((context, event) => {
// No need to refresh the timeline if it is not loaded
if (!context.builds) {
return
}

// When it is a CHECK_REFRESH_TIMELINE workspace event, we want to check if the latest
// build was updated to not over fetch the builds
if (event.type === "CHECK_REFRESH_TIMELINE") {
const latestBuildInTimeline = latestBuild(context.builds)
const isUpdated = event.data?.latest_build.updated_at !== latestBuildInTimeline.updated_at
if (isUpdated) {
return send({ type: "REFRESH_TIMELINE" })
}
} else {
return send({ type: "REFRESH_TIMELINE" })
}
}),
},
guards: {
moreBuildsAvailable,
},
services: {
getWorkspace: async (_, event) => {
Expand All @@ -535,40 +512,55 @@ export const workspaceMachine = createMachine(
throw Error("Cannot get template without workspace")
}
},
startWorkspaceWithLatestTemplate: async (context) => {
startWorkspaceWithLatestTemplate: (context) => async (send) => {
if (context.workspace && context.template) {
return await API.startWorkspace(context.workspace.id, context.template.active_version_id)
const startWorkspacePromise = await API.startWorkspace(
context.workspace.id,
context.template.active_version_id,
)
send({ type: "REFRESH_TIMELINE" })
return startWorkspacePromise
} else {
throw Error("Cannot start workspace without workspace id")
}
},
startWorkspace: async (context) => {
startWorkspace: (context) => async (send) => {
if (context.workspace) {
return await API.startWorkspace(
const startWorkspacePromise = await API.startWorkspace(
context.workspace.id,
context.workspace.latest_build.template_version_id,
)
send({ type: "REFRESH_TIMELINE" })
return startWorkspacePromise
} else {
throw Error("Cannot start workspace without workspace id")
}
},
stopWorkspace: async (context) => {
stopWorkspace: (context) => async (send) => {
if (context.workspace) {
return await API.stopWorkspace(context.workspace.id)
const stopWorkspacePromise = await API.stopWorkspace(context.workspace.id)
send({ type: "REFRESH_TIMELINE" })
return stopWorkspacePromise
} else {
throw Error("Cannot stop workspace without workspace id")
}
},
deleteWorkspace: async (context) => {
if (context.workspace) {
return await API.deleteWorkspace(context.workspace.id)
const deleteWorkspacePromise = await API.deleteWorkspace(context.workspace.id)
send({ type: "REFRESH_TIMELINE" })
return deleteWorkspacePromise
} else {
throw Error("Cannot delete workspace without workspace id")
}
},
cancelWorkspace: async (context) => {
cancelWorkspace: (context) => async (send) => {
if (context.workspace) {
return await API.cancelWorkspaceBuild(context.workspace.latest_build.id)
const cancelWorkspacePromise = await API.cancelWorkspaceBuild(
context.workspace.latest_build.id,
)
send({ type: "REFRESH_TIMELINE" })
return cancelWorkspacePromise
} else {
throw Error("Cannot cancel workspace without build id")
}
Expand All @@ -583,7 +575,7 @@ export const workspaceMachine = createMachine(
// refresh our workspace with each SSE
send({ type: "REFRESH_WORKSPACE", data: JSON.parse(event.data) })
// refresh our timeline
send({ type: "CHECK_REFRESH_TIMELINE", data: JSON.parse(event.data) })
send({ type: "REFRESH_TIMELINE", checkRefresh: true, data: JSON.parse(event.data) })
})

// handle any error events returned by our sse
Expand Down