feat: support model-backend async long run operation (#309)

Because - We need to support model-backend long run operation (#305) This commit - Adapt long run operation request and response - Adjust e2e test - Add reminder to educate our user when they deploy and undeploy model - close #305
instill-ai · Dec 19, 2022 · f795ce8 · f795ce8
1 parent e4a6e22
commit f795ce8
Show file tree

Hide file tree

Showing 18 changed files with 93 additions and 56 deletions.
diff --git a/integration-test/common/mgmt.ts b/integration-test/common/mgmt.ts
@@ -1,6 +1,7 @@
 import { BrowserContext, Page, expect } from "@playwright/test";
 import axios from "axios";
 import { v4 as uuidv4 } from "uuid";
+import { expectToSelectReactSelectOption } from "../helper";
 
 export const removeRegisteredUser = async () => {
   try {
@@ -44,8 +45,12 @@ export const expectToOnboardUser = async (
   await companyField.fill("instill-ai");
 
   // Shoyld select role
-  await page.locator("#role").click({ force: true });
-  await page.locator("#react-select-role-option-0").click();
+  await expectToSelectReactSelectOption(
+    page.locator("#react-select-role-input"),
+    page.locator("data-testid=role-selected-option", {
+      hasText: "Manager (who makes decisions)",
+    })
+  );
 
   // Should accept newsletter subscription
   await page.locator("#newsletterSubscription").check();

diff --git a/integration-test/onboarding.spec.ts b/integration-test/onboarding.spec.ts
@@ -1,5 +1,6 @@
 import { test, expect } from "@playwright/test";
 import { expectToOnboardUser, removeRegisteredUser } from "./common/mgmt";
+import { expectToSelectReactSelectOption } from "./helper";
 
 test("should navigate first time user to the onboarding page", async ({
   page,
@@ -33,8 +34,12 @@ test("should disable start button, if email input format is not correct", async
   await companyField.fill("instill-ai");
 
   // Should select role
-  await page.locator("#role").click({ force: true });
-  await page.locator("#react-select-role-option-0").click();
+  await expectToSelectReactSelectOption(
+    page.locator("#react-select-role-input"),
+    page.locator("data-testid=role-selected-option", {
+      hasText: "Manager (who makes decisions)",
+    })
+  );
 
   // Should disable start button
   const startButton = page.locator("button", { hasText: "Start" });

diff --git a/package.json b/package.json
@@ -25,7 +25,7 @@
   "dependencies": {
     "@amplitude/analytics-browser": "^0.4.1",
     "@code-hike/mdx": "^0.7.3",
-    "@instill-ai/design-system": "^0.0.114",
+    "@instill-ai/design-system": "^0.0.132",
     "@playwright/test": "^1.25.2",
     "axios": "^0.27.2",
     "clsx": "^1.1.1",
@@ -120,5 +120,8 @@
       "eslint --cache --fix"
     ]
   },
+  "engines": {
+    "node": ">=14.6.0"
+  },
   "packageManager": "[email protected]"
 }
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/src/components/destination/ConfigureDestinationForm.tsx b/src/components/destination/ConfigureDestinationForm.tsx
@@ -410,6 +410,7 @@ const ConfigureDestinationForm = ({
             position="mr-auto my-auto"
             type="button"
             color="danger"
+            hoveredShadow={null}
           >
             Delete
           </OutlineButton>

diff --git a/src/components/formik/FormikField/StatefulToggleField/StatefulToggleField.tsx b/src/components/formik/FormikField/StatefulToggleField/StatefulToggleField.tsx
@@ -35,6 +35,7 @@ const StatefulToggleFieldWrapper: FC<ToggleFieldProps & FieldProps> = ({
       error={error}
       onChange={onChange}
       additionalMessageOnLabel={additionalMessageOnLabel}
+      loadingLabelText="Loading..."
     />
   );
 };

diff --git a/src/components/model/ConfigureModelForm.tsx b/src/components/model/ConfigureModelForm.tsx
@@ -221,6 +221,7 @@ const ConfigureModelForm: FC<ConfigureModelFormProps> = ({
             position="mr-auto my-auto"
             type="button"
             color="danger"
+            hoveredShadow={null}
           >
             Delete
           </OutlineButton>

diff --git a/src/components/model/ConfigureModelInstanceForm.tsx b/src/components/model/ConfigureModelInstanceForm.tsx
@@ -18,6 +18,8 @@ const ConfigureModelInstanceForm = ({
   modelInstance,
   marginBottom,
 }: ConfigureModelInstanceFormProps) => {
+  console.log(modelInstance);
+
   const [messageBoxState, setMessageBoxState] =
     useState<ProgressMessageBoxState>({
       activate: false,
@@ -26,8 +28,6 @@ const ConfigureModelInstanceForm = ({
       status: null,
     });
 
-  console.log(modelInstance);
-
   return (
     <FormBase
       padding={null}

diff --git a/src/components/model/CreateModelForm.tsx b/src/components/model/CreateModelForm.tsx
@@ -504,6 +504,7 @@ const CreateModelForm = () => {
               message: error.response?.data.message ?? error.message,
             }));
           } else {
+            console.log(error);
             setDeployModelInstanceMessageBoxState(() => ({
               activate: true,
               status: "error",

diff --git a/src/components/pipeline/ConfigurePipelineForm.tsx b/src/components/pipeline/ConfigurePipelineForm.tsx
@@ -219,6 +219,7 @@ const ConfigurePipelineForm: FC<ConfigurePipelineFormProps> = ({
             position="mr-auto my-auto"
             type="button"
             color="danger"
+            hoveredShadow={null}
           >
             Delete
           </OutlineButton>

diff --git a/src/components/source/ConfigureSourceForm.tsx b/src/components/source/ConfigureSourceForm.tsx
@@ -190,6 +190,7 @@ const ConfigureSourceForm = ({
             position="mr-auto my-auto"
             type="button"
             color="danger"
+            hoveredShadow={null}
           >
             Delete
           </OutlineButton>

diff --git a/src/components/ui/ChangeResourceStateButton.tsx b/src/components/ui/ChangeResourceStateButton.tsx
@@ -5,17 +5,18 @@ import { FC, useState, useEffect, useCallback } from "react";
 import { UseMutationResult } from "react-query";
 import cn from "clsx";
 import { AxiosError } from "axios";
+import { Operation } from "@/lib/instill/types";
 
 export type ChangeResourceStateButtonProps = {
   resource: Nullable<ModelInstance | Pipeline>;
   switchOff: UseMutationResult<
-    ModelInstance | Pipeline,
+    { modelInstance: ModelInstance; operation: Operation } | Pipeline,
     unknown,
     string,
     unknown
   >;
   switchOn: UseMutationResult<
-    ModelInstance | Pipeline,
+    { modelInstance: ModelInstance; operation: Operation } | Pipeline,
     unknown,
     string,
     unknown
@@ -29,28 +30,21 @@ const ChangeResourceStateButton: FC<ChangeResourceStateButtonProps> = ({
   switchOff,
   marginBottom,
 }) => {
-  const [isChanging, setIsChanging] = useState(false);
   const [error, setError] = useState<Nullable<string>>(null);
 
   useEffect(() => {
     setError(null);
   }, [resource]);
 
   const changeResourceStateHandler = useCallback(() => {
-    if (!resource) return;
-
-    setIsChanging(true);
+    if (!resource || resource.state === "STATE_UNSPECIFIED") return;
 
     if (
       resource.state === "STATE_ONLINE" ||
       resource.state === "STATE_ACTIVE"
     ) {
       switchOff.mutate(resource.name, {
-        onSuccess: () => {
-          setIsChanging(false);
-        },
         onError: (error) => {
-          setIsChanging(false);
           if (error instanceof AxiosError) {
             setError(
               error.response?.data.message ??
@@ -63,11 +57,7 @@ const ChangeResourceStateButton: FC<ChangeResourceStateButtonProps> = ({
       });
     } else {
       switchOn.mutate(resource.name, {
-        onSuccess: () => {
-          setIsChanging(false);
-        },
         onError: (error) => {
-          setIsChanging(false);
           if (error instanceof AxiosError) {
             setError(
               error.response?.data.message ??
@@ -82,7 +72,7 @@ const ChangeResourceStateButton: FC<ChangeResourceStateButtonProps> = ({
   }, [switchOn, switchOff, resource]);
 
   return (
-    <div className={cn(marginBottom)}>
+    <div className={cn("flex flex-row", marginBottom)}>
       <StatefulToggleField
         id="pipelineStateToggleButton"
         value={
@@ -93,11 +83,14 @@ const ChangeResourceStateButton: FC<ChangeResourceStateButtonProps> = ({
         }
         onChange={changeResourceStateHandler}
         label="State"
-        additionalMessageOnLabel={null}
         error={error}
         state={
-          isChanging ? "STATE_LOADING" : resource?.state || "STATE_UNSPECIFIED"
+          resource?.state === "STATE_UNSPECIFIED"
+            ? "STATE_LOADING"
+            : resource?.state || "STATE_UNSPECIFIED"
         }
+        disabled={resource?.state === "STATE_UNSPECIFIED"}
+        loadingLabelText="Model instance is in the long running operation, please refresh this page to get the new status"
       />
     </div>
   );

diff --git a/src/components/ui/Modals/DeleteResourceModal/DeleteResourceModal.tsx b/src/components/ui/Modals/DeleteResourceModal/DeleteResourceModal.tsx
@@ -152,6 +152,7 @@ const DeleteResourceModal = ({
             disabled={false}
             onClickHandler={() => setModalIsOpen(false)}
             color="secondary"
+            hoveredShadow={null}
           >
             <p className="mx-auto">Cancel</p>
           </OutlineButton>
@@ -160,6 +161,7 @@ const DeleteResourceModal = ({
             onClickHandler={handleDeleteResource}
             color="danger"
             disabled={canDeleteResource ? false : true}
+            hoveredShadow={null}
           >
             <p className="mx-auto">Delete</p>
           </OutlineButton>

diff --git a/src/lib/instill/model/actions.ts b/src/lib/instill/model/actions.ts
@@ -1,23 +1,23 @@
 import axios from "axios";
-import { ModelInstance } from "./types";
+import { Operation } from "../types";
 
 export type DeployModelResponse = {
-  instance: ModelInstance;
+  operation: Operation;
 };
 
 export const deployModelInstanceAction = async (modelInstanceName: string) => {
   try {
     const { data } = await axios.post<DeployModelResponse>(
       `${process.env.NEXT_PUBLIC_MODEL_BACKEND_BASE_URL}/${process.env.NEXT_PUBLIC_API_VERSION}/${modelInstanceName}/deploy`
     );
-    return Promise.resolve(data.instance);
+    return Promise.resolve(data.operation);
   } catch (err) {
     return Promise.reject(err);
   }
 };
 
 export type UnDeployModelResponse = {
-  instance: ModelInstance;
+  operation: Operation;
 };
 
 export const unDeployModelInstanceAction = async (
@@ -27,7 +27,7 @@ export const unDeployModelInstanceAction = async (
     const { data } = await axios.post<UnDeployModelResponse>(
       `${process.env.NEXT_PUBLIC_MODEL_BACKEND_BASE_URL}/${process.env.NEXT_PUBLIC_API_VERSION}/${modelInstanceName}/undeploy`
     );
-    return Promise.resolve(data.instance);
+    return Promise.resolve(data.operation);
   } catch (err) {
     return Promise.reject(err);
   }

diff --git a/src/lib/instill/model/queries.ts b/src/lib/instill/model/queries.ts
@@ -102,7 +102,7 @@ export const getModelInstanceQuery = async (
 ): Promise<ModelInstance> => {
   try {
     const { data } = await axios.get<GetModelInstanceResponse>(
-      `${process.env.NEXT_PUBLIC_MODEL_BACKEND_BASE_URL}/${process.env.NEXT_PUBLIC_API_VERSION}/${modelInstanceName}`
+      `${process.env.NEXT_PUBLIC_MODEL_BACKEND_BASE_URL}/${process.env.NEXT_PUBLIC_API_VERSION}/${modelInstanceName}?view=VIEW_FULL`
     );
 
     return Promise.resolve(data.instance);

diff --git a/src/lib/instill/types.ts b/src/lib/instill/types.ts
@@ -9,3 +9,10 @@ export type Violation = {
   description: string;
   subject: string;
 };
+
+export type Operation = {
+  name: string;
+  response: any;
+  metadata: any;
+  done: boolean;
+};
diff --git a/src/services/model/useDeployModelInstance.ts b/src/services/model/useDeployModelInstance.ts
@@ -1,32 +1,43 @@
 import { useMutation, useQueryClient } from "react-query";
-import { deployModelInstanceAction, ModelInstance } from "@/lib/instill";
+import {
+  deployModelInstanceAction,
+  ModelInstance,
+  getModelInstanceQuery,
+} from "@/lib/instill";
 
 const useDeployModelInstance = () => {
   const queryClient = useQueryClient();
   return useMutation(
     async (modelInstanceName: string) => {
-      const modelInstance = await deployModelInstanceAction(modelInstanceName);
-      return Promise.resolve(modelInstance);
+      try {
+        const operation = await deployModelInstanceAction(modelInstanceName);
+
+        // Get the current model instance staus
+        const modelInstance = await getModelInstanceQuery(modelInstanceName);
+        return Promise.resolve({ modelInstance, operation });
+      } catch (err) {
+        return Promise.reject(err);
+      }
     },
     {
-      onSuccess: (newModelInstance) => {
-        const modelId = newModelInstance.name.split("/")[1];
+      onSuccess: ({ modelInstance }) => {
+        const modelId = modelInstance.name.split("/")[1];
 
         queryClient.setQueryData<ModelInstance>(
-          ["models", modelId, "modelInstances", newModelInstance.id],
-          newModelInstance
+          ["models", modelId, "modelInstances", modelInstance.id],
+          modelInstance
         );
 
         queryClient.setQueryData<ModelInstance[]>(
           ["models", modelId, "modelInstances"],
           (old) => {
             if (!old) {
-              return [newModelInstance];
+              return [modelInstance];
             }
 
             return [
-              ...old.filter((e) => e.id !== newModelInstance.id),
-              newModelInstance,
+              ...old.filter((e) => e.id !== modelInstance.id),
+              modelInstance,
             ];
           }
         );