refactor: integrate react-query for execution data fetching (#123)

* refactor: using react-query to load top level Execution * refactor: upgrading react-query and fixing execution termination * refactor: handle 401s on queries and do auth flow * refactor: adding conditional refresh for execution status * refactor: cleanup broken files after context refactor * chore: docs
flyteorg · Jan 4, 2021 · 1de2618 · 1de2618
1 parent f2317d5
commit 1de2618
Show file tree

Hide file tree

Showing 21 changed files with 436 additions and 120 deletions.
diff --git a/package.json b/package.json
@@ -177,6 +177,8 @@
     "react-intersection-observer": "^8.25.1",
     "react-json-tree": "^0.11.0",
     "react-loading-skeleton": "^1.1.2",
+    "react-query": "^3.2.0-beta",
+    "react-query-devtools": "^3.0.0-beta",
     "react-router": "^5.0.1",
     "react-router-dom": "^5.0.1",
     "react-test-renderer": "^16.9.0",

diff --git a/src/components/App/App.tsx b/src/components/App/App.tsx
@@ -2,6 +2,8 @@ import { CssBaseline } from '@material-ui/core';
 import { ThemeProvider } from '@material-ui/styles';
 import { env } from 'common/env';
 import { debug, debugPrefix } from 'common/log';
+import { QueryAuthorizationObserver } from 'components/common/QueryAuthorizationObserver';
+import { queryClient } from 'components/common/queryCache';
 import { APIContext, useAPIState } from 'components/data/apiContext';
 import { LoginExpiredHandler } from 'components/Errors/LoginExpiredHandler';
 import { SystemStatusBanner } from 'components/Notifications/SystemStatusBanner';
@@ -11,6 +13,8 @@ import * as React from 'react';
 import { Helmet } from 'react-helmet';
 import { hot } from 'react-hot-loader';
 import { SkeletonTheme } from 'react-loading-skeleton';
+import { QueryClientProvider } from 'react-query';
+import { ReactQueryDevtools } from 'react-query-devtools';
 import { Router } from 'react-router-dom';
 import { ApplicationRouter } from 'routes/ApplicationRouter';
 import { history } from 'routes/history';
@@ -25,26 +29,33 @@ export const AppComponent: React.StatelessComponent<{}> = () => {
 
     return (
         <ThemeProvider theme={muiTheme}>
-            <APIContext.Provider value={apiState}>
-                <SkeletonTheme
-                    color={skeletonColor}
-                    highlightColor={skeletonHighlightColor}
-                >
-                    <CssBaseline />
-                    <Helmet>
-                        <title>Flyte Console</title>
-                        <meta name="viewport" content="width=device-width" />
-                    </Helmet>
-                    <Router history={history}>
-                        <ErrorBoundary fixed={true}>
-                            <NavBarRouter />
-                            <ApplicationRouter />
-                            <LoginExpiredHandler />
-                        </ErrorBoundary>
-                    </Router>
-                    <SystemStatusBanner />
-                </SkeletonTheme>
-            </APIContext.Provider>
+            <QueryClientProvider client={queryClient}>
+                <APIContext.Provider value={apiState}>
+                    <QueryAuthorizationObserver />
+                    <SkeletonTheme
+                        color={skeletonColor}
+                        highlightColor={skeletonHighlightColor}
+                    >
+                        <CssBaseline />
+                        <Helmet>
+                            <title>Flyte Console</title>
+                            <meta
+                                name="viewport"
+                                content="width=device-width"
+                            />
+                        </Helmet>
+                        <Router history={history}>
+                            <ErrorBoundary fixed={true}>
+                                <NavBarRouter />
+                                <ApplicationRouter />
+                                <LoginExpiredHandler />
+                            </ErrorBoundary>
+                        </Router>
+                        <SystemStatusBanner />
+                    </SkeletonTheme>
+                </APIContext.Provider>
+                <ReactQueryDevtools initialIsOpen={false} />
+            </QueryClientProvider>
         </ThemeProvider>
     );
 };

diff --git a/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx b/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx
@@ -2,14 +2,15 @@ import { Collapse, IconButton } from '@material-ui/core';
 import { makeStyles, Theme } from '@material-ui/core/styles';
 import ExpandMore from '@material-ui/icons/ExpandMore';
 import * as classnames from 'classnames';
-import { WaitForData, withRouteParams } from 'components/common';
-import { RefreshConfig, useDataRefresher } from 'components/hooks';
+import { withRouteParams } from 'components/common';
+import { WaitForQuery } from 'components/common/WaitForQuery';
+import { RefreshConfig } from 'components/hooks';
 import { Execution } from 'models';
 import * as React from 'react';
 import { executionRefreshIntervalMs } from '../constants';
 import { ExecutionContext, ExecutionDataCacheContext } from '../contexts';
 import { useExecutionDataCache } from '../useExecutionDataCache';
-import { useWorkflowExecution } from '../useWorkflowExecution';
+import { useWorkflowExecutionQuery } from '../useWorkflowExecution';
 import { executionIsTerminal } from '../utils';
 import { ExecutionDetailsAppBarContent } from './ExecutionDetailsAppBarContent';
 import { ExecutionMetadata } from './ExecutionMetadata';
@@ -50,8 +51,47 @@ const executionRefreshConfig: RefreshConfig<Execution> = {
     valueIsFinal: executionIsTerminal
 };
 
+interface RenderExecutionDetailsProps {
+    execution: Execution;
+}
+
+const RenderExecutionDetails: React.FC<RenderExecutionDetailsProps> = ({
+    execution
+}) => {
+    const styles = useStyles();
+    const [metadataExpanded, setMetadataExpanded] = React.useState(true);
+    const toggleMetadata = () => setMetadataExpanded(!metadataExpanded);
+    const dataCache = useExecutionDataCache();
+    const contextValue = {
+        execution
+    };
+
+    return (
+        <ExecutionContext.Provider value={contextValue}>
+            <ExecutionDetailsAppBarContent execution={execution} />
+            <div className={styles.metadataContainer}>
+                <Collapse in={metadataExpanded}>
+                    <ExecutionMetadata execution={execution} />
+                </Collapse>
+                <div className={styles.expandCollapseContainer}>
+                    <IconButton size="small" onClick={toggleMetadata}>
+                        <ExpandMore
+                            className={classnames(styles.expandCollapseButton, {
+                                expanded: metadataExpanded
+                            })}
+                        />
+                    </IconButton>
+                </div>
+            </div>
+            <ExecutionDataCacheContext.Provider value={dataCache}>
+                <ExecutionNodeViews execution={execution} />
+            </ExecutionDataCacheContext.Provider>
+        </ExecutionContext.Provider>
+    );
+};
+
 /** The view component for the Execution Details page */
-export const ExecutionDetailsContainer: React.FC<ExecutionDetailsRouteParams> = ({
+export const ExecutionDetailsContainer: React.FC<ExecutionDetailsProps> = ({
     executionId,
     domainId,
     projectId
@@ -61,46 +101,15 @@ export const ExecutionDetailsContainer: React.FC<ExecutionDetailsRouteParams> =
         domain: domainId,
         name: executionId
     };
-    const styles = useStyles();
-    const [metadataExpanded, setMetadataExpanded] = React.useState(true);
-    const toggleMetadata = () => setMetadataExpanded(!metadataExpanded);
-    const dataCache = useExecutionDataCache();
-    const { fetchable, terminateExecution } = useWorkflowExecution(
-        id,
-        dataCache
+
+    const renderExecutionDetails = (execution: Execution) => (
+        <RenderExecutionDetails execution={execution} />
     );
-    useDataRefresher(id, fetchable, executionRefreshConfig);
-    const contextValue = {
-        terminateExecution,
-        execution: fetchable.value
-    };
 
     return (
-        <WaitForData {...fetchable}>
-            <ExecutionContext.Provider value={contextValue}>
-                <ExecutionDetailsAppBarContent execution={fetchable.value} />
-                <div className={styles.metadataContainer}>
-                    <Collapse in={metadataExpanded}>
-                        <ExecutionMetadata execution={fetchable.value} />
-                    </Collapse>
-                    <div className={styles.expandCollapseContainer}>
-                        <IconButton size="small" onClick={toggleMetadata}>
-                            <ExpandMore
-                                className={classnames(
-                                    styles.expandCollapseButton,
-                                    {
-                                        expanded: metadataExpanded
-                                    }
-                                )}
-                            />
-                        </IconButton>
-                    </div>
-                </div>
-                <ExecutionDataCacheContext.Provider value={dataCache}>
-                    <ExecutionNodeViews execution={fetchable.value} />
-                </ExecutionDataCacheContext.Provider>
-            </ExecutionContext.Provider>
-        </WaitForData>
+        <WaitForQuery query={useWorkflowExecutionQuery(id)}>
+            {renderExecutionDetails}
+        </WaitForQuery>
     );
 };
 

diff --git a/src/components/Executions/ExecutionDetails/ExecutionDetailsAppBarContent.tsx b/src/components/Executions/ExecutionDetails/ExecutionDetailsAppBarContent.tsx
@@ -14,7 +14,7 @@ import { Link as RouterLink } from 'react-router-dom';
 import { ExecutionInputsOutputsModal } from '../ExecutionInputsOutputsModal';
 import { ExecutionStatusBadge } from '../ExecutionStatusBadge';
 import { TerminateExecutionButton } from '../TerminateExecution';
-import { executionIsTerminal } from '../utils';
+import { executionIsRunning, executionIsTerminal } from '../utils';
 import { backLinkTitle, executionActionStrings } from './constants';
 import { RelaunchExecutionForm } from './RelaunchExecutionForm';
 import { getExecutionBackLink, getExecutionSourceId } from './utils';
@@ -80,6 +80,7 @@ export const ExecutionDetailsAppBarContent: React.FC<{
     const { phase } = execution.closure;
     const sourceId = getExecutionSourceId(execution);
     const { backLink = getExecutionBackLink(execution) } = useLocationState();
+    const isRunning = executionIsRunning(execution);
     const isTerminal = executionIsTerminal(execution);
     const onClickShowInputsOutputs = () => setShowInputsOutputs(true);
     const onClickRelaunch = () => setShowRelaunchForm(true);
@@ -96,7 +97,9 @@ export const ExecutionDetailsAppBarContent: React.FC<{
         );
     }
 
-    const actionContent = isTerminal ? (
+    const actionContent = isRunning ? (
+        <TerminateExecutionButton className={styles.actionButton} />
+    ) : isTerminal ? (
         <Button
             variant="outlined"
             color="primary"
@@ -109,12 +112,9 @@ export const ExecutionDetailsAppBarContent: React.FC<{
         >
             Relaunch
         </Button>
-    ) : (
-        <TerminateExecutionButton className={styles.actionButton} />
-    );
+    ) : null;
 
-    // For running executions, add an overflow menu with the ability to clone
-    // while we are still running.
+    // For non-terminal executions, add an overflow menu with the ability to clone
     const moreActionsContent = !isTerminal ? (
         <MoreOptionsMenu
             className={styles.moreActions}

diff --git a/src/components/Executions/ExecutionDetails/test/ExecutionDetailsAppBarContent.test.tsx b/src/components/Executions/ExecutionDetails/test/ExecutionDetailsAppBarContent.test.tsx
@@ -26,20 +26,14 @@ jest.mock('components/Navigation/NavBarContent', () => ({
 describe('ExecutionDetailsAppBarContent', () => {
     let execution: Execution;
     let executionContext: ExecutionContextData;
-    let mockTerminateExecution: jest.Mock<Promise<void>>;
-    let terminatePromise: DelayedPromiseResult<void>;
     let sourceId: Identifier;
 
     beforeEach(() => {
         execution = createMockExecution();
         sourceId = execution.closure.workflowId;
-        mockTerminateExecution = jest.fn().mockImplementation(() => {
-            terminatePromise = delayedPromise();
-            return terminatePromise;
-        });
+
         executionContext = {
-            execution,
-            terminateExecution: mockTerminateExecution
+            execution
         };
     });
 

diff --git a/src/components/Executions/ExecutionDetails/test/ExecutionNodeViews.test.tsx b/src/components/Executions/ExecutionDetails/test/ExecutionNodeViews.test.tsx
@@ -85,8 +85,7 @@ describe('ExecutionNodeViews', () => {
         );
 
         executionContext = {
-            execution: workflowExecution,
-            terminateExecution: jest.fn().mockRejectedValue('Not Implemented')
+            execution: workflowExecution
         };
 
         props = { execution: workflowExecution };

diff --git a/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx b/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx
@@ -167,8 +167,7 @@ describe('NodeExecutionsTable', () => {
         );
 
         executionContext = {
-            execution: mockExecution,
-            terminateExecution: jest.fn().mockRejectedValue('Not Implemented')
+            execution: mockExecution
         };
     });
 

diff --git a/src/components/Executions/TerminateExecution/TerminateExecutionForm.tsx b/src/components/Executions/TerminateExecution/TerminateExecutionForm.tsx
@@ -40,8 +40,7 @@ export const TerminateExecutionForm: React.FC<{
     const {
         cause,
         setCause,
-        terminating,
-        terminationError,
+        terminationState: { error, isLoading: terminating },
         terminateExecution
     } = useTerminateExecutionState(onClose);
 
@@ -74,9 +73,7 @@ export const TerminateExecutionForm: React.FC<{
                     value={cause}
                 />
             </FormControl>
-            {terminationError && (
-                <p className={commonStyles.errorText}>{terminationError}</p>
-            )}
+            {error && <p className={commonStyles.errorText}>{`${error}`}</p>}
             <div className={commonStyles.formButtonGroup}>
                 <Button
                     color="primary"

diff --git a/src/components/Executions/TerminateExecution/test/createMockTerminateExecutionState.ts b/src/components/Executions/TerminateExecution/test/createMockTerminateExecutionState.ts