Skip to content

Commit

Permalink
[IA-4667] isLoadingCloudEnvironments flag passed with workspace wrapp…
Browse files Browse the repository at this point in the history
…er (#4587)
  • Loading branch information
mlilynolting authored Feb 6, 2024
1 parent 8d45d9d commit 792941e
Show file tree
Hide file tree
Showing 20 changed files with 139 additions and 40 deletions.
1 change: 1 addition & 0 deletions src/analysis/Analyses.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const defaultAnalysesData: AnalysesData = {
lastRefresh: null,
runtimes: [],
refreshRuntimes: () => Promise.resolve(),
isLoadingCloudEnvironments: false,
appDataDisks: [],
persistentDisks: [],
};
Expand Down
11 changes: 10 additions & 1 deletion src/analysis/Analyses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,15 @@ export interface SortOrderInfo {
export const BaseAnalyses = (
{
workspace,
analysesData: { apps, refreshApps, runtimes, refreshRuntimes, appDataDisks, persistentDisks },
analysesData: {
apps,
refreshApps,
runtimes,
refreshRuntimes,
appDataDisks,
persistentDisks,
isLoadingCloudEnvironments,
},
storageDetails: { googleBucketLocation, azureContainerRegion },
onRequesterPaysError,
}: AnalysesProps,
Expand Down Expand Up @@ -752,6 +760,7 @@ export const BaseAnalyses = (
runtimes,
persistentDisks,
refreshRuntimes,
isLoadingCloudEnvironments,
appDataDisks,
refreshAnalyses,
analyses,
Expand Down
5 changes: 3 additions & 2 deletions src/analysis/AnalysisLauncher.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const AnalysisLauncher = _.flow(
analysisName,
workspace,
workspace: { accessLevel, canCompute },
analysesData: { runtimes, refreshRuntimes, persistentDisks },
analysesData: { runtimes, refreshRuntimes, persistentDisks, isLoadingCloudEnvironments },
storageDetails: { googleBucketLocation, azureContainerRegion },
},
_ref
Expand Down Expand Up @@ -117,6 +117,7 @@ const AnalysisLauncher = _.flow(
workspace,
setCreateOpen,
refreshRuntimes,
isLoadingCloudEnvironments,
readOnlyAccess: !(canWrite(accessLevel) && canCompute),
}),
h(AnalysisPreviewFrame, { styles: iframeStyles, analysisName, toolLabel: currentFileToolLabel, workspace }),
Expand Down Expand Up @@ -166,7 +167,7 @@ const AnalysisLauncher = _.flow(
setCreateOpen(false);
},
}),
busy && spinnerOverlay,
(busy || isLoadingCloudEnvironments) && spinnerOverlay,
]),
]);
}
Expand Down
2 changes: 2 additions & 0 deletions src/analysis/ContextBar.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ const contextBarProps: ContextBarProps = {
refreshRuntimes: () => Promise.resolve(),
storageDetails: defaultGoogleBucketOptions,
refreshApps: () => Promise.resolve(),
isLoadingCloudEnvironments: false,
workspace: defaultGoogleWorkspace,
};

Expand All @@ -423,6 +424,7 @@ const contextBarPropsForAzure: ContextBarProps = {
refreshRuntimes: () => Promise.resolve(),
storageDetails: { ...defaultGoogleBucketOptions, ...populatedAzureStorageOptions },
refreshApps: () => Promise.resolve(),
isLoadingCloudEnvironments: false,
workspace: defaultAzureWorkspace,
};

Expand Down
3 changes: 3 additions & 0 deletions src/analysis/ContextBar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export interface ContextBarProps {
refreshRuntimes: (maybeStale?: boolean) => Promise<void>;
storageDetails: StorageDetails;
refreshApps: (maybeStale?: boolean) => Promise<void>;
isLoadingCloudEnvironments: boolean;
workspace: BaseWorkspace;
persistentDisks: PersistentDisk[];
}
Expand All @@ -96,6 +97,7 @@ export const ContextBar = ({
refreshRuntimes,
storageDetails,
refreshApps,
isLoadingCloudEnvironments,
workspace,
persistentDisks,
}: ContextBarProps) => {
Expand Down Expand Up @@ -248,6 +250,7 @@ export const ContextBar = ({
appDataDisks,
refreshRuntimes,
refreshApps,
isLoadingCloudEnvironments,
workspace,
canCompute,
persistentDisks,
Expand Down
1 change: 1 addition & 0 deletions src/analysis/modals/AnalysisModal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const defaultGcpModalProps: AnalysisModalProps = {
apps: [] as App[],
appDataDisks: [],
persistentDisks: [],
isLoadingCloudEnvironments: false,
onDismiss: () => {},
onError: () => {},
onSuccess: () => {},
Expand Down
36 changes: 20 additions & 16 deletions src/analysis/modals/AnalysisModal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export interface AnalysisModalProps {
apps: App[] | undefined;
appDataDisks: AppDataDisk[] | undefined;
persistentDisks: PersistentDisk[] | undefined;
isLoadingCloudEnvironments: boolean;
onDismiss: () => void;
onError: () => void;
onSuccess: () => void;
Expand All @@ -94,6 +95,7 @@ export const AnalysisModal = withDisplayName('AnalysisModal')(
onError,
onSuccess,
runtimes,
isLoadingCloudEnvironments,
apps,
appDataDisks,
persistentDisks,
Expand All @@ -110,18 +112,17 @@ export const AnalysisModal = withDisplayName('AnalysisModal')(
const prevAnalysisName = usePrevious(analysisName);
const [currentToolObj, setCurrentToolObj] = useState<Tool>();
const [fileExt, setFileExt] = useState('');
const currentTool: ToolLabel | undefined = currentToolObj?.label;

const { loadedState, createAnalysis, pendingCreate } = analysisFileStore;
const loading =
isLoadingCloudEnvironments || loadedState.status === 'Loading' || pendingCreate.status === 'Loading';

const currentTool: ToolLabel | undefined = currentToolObj?.label;
const currentRuntime: Runtime | undefined = getCurrentRuntime(runtimes);
const currentDisk = getCurrentPersistentDisk(runtimes, persistentDisks);
const currentRuntimeToolLabel = currentRuntime && getToolLabelFromCloudEnv(currentRuntime);
const currentApp = (toolLabel: AppToolLabel): App | undefined => getCurrentApp(toolLabel, apps);

// TODO: Bring in as props from Analyses OR bring entire AnalysisFileStore from props.
const { loadedState, createAnalysis, pendingCreate } = analysisFileStore;
// TODO: When the above is done, this check below may not be necessary.
const analyses = loadedState.status !== 'None' ? loadedState.state : null;
const status = loadedState.status;
const resetView = () => {
setViewMode(undefined);
setAnalysisName('');
Expand Down Expand Up @@ -220,6 +221,7 @@ export const AnalysisModal = withDisplayName('AnalysisModal')(
tool: currentTool,
currentRuntime,
currentDisk,
isLoadingCloudEnvironments,
onDismiss,
onError,
onSuccess,
Expand All @@ -232,6 +234,7 @@ export const AnalysisModal = withDisplayName('AnalysisModal')(
tool: currentTool,
currentRuntime,
currentDisk: persistentDisks ? persistentDisks[0] : undefined,
isLoadingCloudEnvironments,
onDismiss,
onSuccess,
});
Expand Down Expand Up @@ -297,6 +300,8 @@ export const AnalysisModal = withDisplayName('AnalysisModal')(
Clickable,
{
style: styles.toolCard,
disabled: loading,
tooltip: loading ? 'Loading' : runtimeTool.label,
onClick: () => {
setCurrentToolObj(runtimeTool);
setFileExt(runtimeTool.defaultExt);
Expand Down Expand Up @@ -325,8 +330,8 @@ export const AnalysisModal = withDisplayName('AnalysisModal')(
enterNextViewMode(appTool.label);
},
hover: !currentApp ? styles.hover : undefined,
disabled: !!currentApp,
tooltip: currentApp ? appDisabledMessages[appTool.label] : '',
disabled: !!currentApp || loading,
tooltip: currentApp ? appDisabledMessages[appTool.label] : loading ? 'Loading' : '',
key: appTool.label,
},
[toolImages[appTool.label]]
Expand Down Expand Up @@ -355,6 +360,7 @@ export const AnalysisModal = withDisplayName('AnalysisModal')(
accept: `.${runtimeTools.Jupyter.ext.join(', .')}, .${runtimeTools.RStudio.ext.join(', .')}`,
style: { flexGrow: 1, backgroundColor: colors.light(), height: '100%' },
activeStyle: { backgroundColor: colors.accent(0.2), cursor: 'copy' },
disabled: loading,
onDropRejected: () =>
reportError(
'Not a valid analysis file',
Expand Down Expand Up @@ -446,7 +452,9 @@ export const AnalysisModal = withDisplayName('AnalysisModal')(
const errors = validate(
{ analysisName: `${analysisName}.${fileExt}`, notebookKernel },
{
analysisName: analysisNameValidator(_.map(({ name }) => getFileName(name), analyses)),
analysisName: analysisNameValidator(
_.map(({ name }) => getFileName(name), loadedState.status === 'None' ? [] : loadedState.state)
),
notebookKernel: { presence: { allowEmpty: true } },
}
);
Expand Down Expand Up @@ -509,9 +517,8 @@ export const AnalysisModal = withDisplayName('AnalysisModal')(
h(
ButtonPrimary,
{
// TODO: See spinner overlay comment. Change to pendingCreate.status === 'Loading' || errors.
disabled: status === 'Loading' || pendingCreate.status === 'Loading' || errors,
tooltip: Utils.summarizeErrors(errors),
disabled: loading || errors,
tooltip: errors ? Utils.summarizeErrors(errors) : loading ? 'Loading' : '',
onClick: async () => {
try {
const contents = Utils.cond(
Expand All @@ -536,10 +543,7 @@ export const AnalysisModal = withDisplayName('AnalysisModal')(
['Create Analysis']
),
]),
// TODO: Once Analyses.js is converted to implement useAnalysisFiles and refresh is called within create,
// change next line to pendingCreate.status === 'Loading' && spinnerOverlay
// Currently this will be close enough to the desired functionality.
(status === 'Loading' || pendingCreate.status === 'Loading') && spinnerOverlay,
loading && spinnerOverlay,
]);
};

Expand Down
4 changes: 4 additions & 0 deletions src/analysis/modals/CloudEnvironmentModal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const CloudEnvironmentModal = ({
appDataDisks,
refreshRuntimes,
refreshApps,
isLoadingCloudEnvironments,
workspace,
persistentDisks,
// Note: for Azure environments `location` and `computeRegion` are identical
Expand All @@ -88,6 +89,7 @@ export const CloudEnvironmentModal = ({
appDataDisks: PersistentDisk[];
refreshRuntimes: () => Promise<void>;
refreshApps: () => Promise<void>;
isLoadingCloudEnvironments: boolean;
workspace: BaseWorkspace;
persistentDisks: PersistentDisk[];
location: string;
Expand Down Expand Up @@ -115,6 +117,7 @@ export const CloudEnvironmentModal = ({
tool,
currentRuntime,
currentDisk,
isLoadingCloudEnvironments,
location,
onDismiss,
onSuccess,
Expand All @@ -128,6 +131,7 @@ export const CloudEnvironmentModal = ({
workspace,
currentRuntime,
currentDisk: getReadyPersistentDisk(persistentDisks),
isLoadingCloudEnvironments,
location,
tool,
onDismiss,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import _ from 'lodash/fp';
import { Fragment, useState } from 'react';
import { Fragment, useEffect, useState } from 'react';
import { div, h, label, p, span } from 'react-hyperscript-helpers';
import { AboutPersistentDiskView } from 'src/analysis/modals/ComputeModal/AboutPersistentDiskView';
import { AzurePersistentDiskSection } from 'src/analysis/modals/ComputeModal/AzureComputeModal/AzurePersistentDiskSection';
Expand Down Expand Up @@ -48,11 +48,13 @@ export const AzureComputeModalBase = ({
workspace,
currentRuntime,
currentDisk,
isLoadingCloudEnvironments,
location,
tool,
hideCloseButton = false,
}) => {
const [loading, setLoading] = useState(false);
const [_loading, setLoading] = useState(false);
const loading = _loading || isLoadingCloudEnvironments;
const [viewMode, setViewMode] = useState(undefined);
const [currentRuntimeDetails, setCurrentRuntimeDetails] = useState(currentRuntime);
const [currentPersistentDiskDetails] = useState(currentDisk);
Expand All @@ -64,15 +66,21 @@ export const AzureComputeModalBase = ({
const hasGpu = () => !!azureMachineTypes[computeConfig.machineType]?.hasGpu;
// Lifecycle
useOnMount(() => {
const loadCloudEnvironment = _.flow(
withErrorReportingInModal('Error loading cloud environment', onError),
Utils.withBusyState(setLoading)
)(async () => {
const runtimeDetails = currentRuntime ? await Ajax().Runtimes.runtimeV2(workspaceId, currentRuntime.runtimeName).details() : null;
const announceModalOpen = async () => {
Ajax().Metrics.captureEvent(Events.cloudEnvironmentConfigOpen, {
existingConfig: !!currentRuntime,
...extractWorkspaceDetails(workspace.workspace),
});
};
announceModalOpen();
});

useEffect(() => {
const refreshRuntime = _.flow(
withErrorReportingInModal('Error loading cloud environment', onError),
Utils.withBusyState(setLoading)
)(async () => {
const runtimeDetails = currentRuntime ? await Ajax().Runtimes.runtimeV2(workspaceId, currentRuntime.runtimeName).details() : null;
setCurrentRuntimeDetails(runtimeDetails);
setComputeConfig({
machineType: runtimeDetails?.runtimeConfig?.machineType || defaultAzureMachineType,
Expand All @@ -83,8 +91,8 @@ export const AzureComputeModalBase = ({
autopauseThreshold: runtimeDetails ? runtimeDetails.autopauseThreshold || autopauseDisabledValue : defaultAutopauseThreshold,
});
});
loadCloudEnvironment();
});
refreshRuntime();
}, [currentRuntime, location, onError, workspaceId, setCurrentRuntimeDetails, setComputeConfig]);

const renderTitleAndTagline = () => {
return h(Fragment, [
Expand All @@ -106,6 +114,7 @@ export const AzureComputeModalBase = ({
ButtonOutline,
{
onClick: () => setViewMode('deleteEnvironment'),
disabled: loading,
},
[
Utils.cond(
Expand Down Expand Up @@ -280,8 +289,11 @@ export const AzureComputeModalBase = ({
const renderActionButton = () => {
const commonButtonProps = {
tooltipSide: 'left',
disabled: Utils.cond([viewMode === 'deleteEnvironment', () => getIsRuntimeBusy(currentRuntimeDetails)], () => doesRuntimeExist()),
disabled: Utils.cond([loading, true], [viewMode === 'deleteEnvironment', () => getIsRuntimeBusy(currentRuntimeDetails)], () =>
doesRuntimeExist()
),
tooltip: Utils.cond(
[loading, 'Loading cloud environments'],
[viewMode === 'deleteEnvironment', () => (getIsRuntimeBusy(currentRuntimeDetails) ? 'Cannot delete a runtime while it is busy' : undefined)],
[doesRuntimeExist(), () => 'Update not supported for azure runtimes'],
() => undefined
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const defaultAjaxImpl = {
};

const verifyEnabled = (item) => expect(item).not.toHaveAttribute('disabled');
const verifyDisabled = (item) => expect(item).toHaveAttribute('disabled');

describe('AzureComputeModal', () => {
beforeAll(() => {});
Expand Down Expand Up @@ -93,6 +94,19 @@ describe('AzureComputeModal', () => {
expect(deleteButton).toBeNull();
});

it('disables submit while loading', async () => {
// Arrange

// Act
await act(async () => {
render(h(AzureComputeModalBase, { ...defaultModalProps, isLoadingCloudEnvironments: true }));
});

// Assert
verifyDisabled(getCreateButton());
screen.getByText('Azure Cloud Environment');
});

it('sends the proper leo API call in default create case (no runtimes or disks)', async () => {
// Arrange
const user = userEvent.setup();
Expand Down
Loading

0 comments on commit 792941e

Please sign in to comment.