Skip to content

Commit

Permalink
Merge pull request opendatahub-io#2942 from DaoDaoNoCode/jira-rhoaien…
Browse files Browse the repository at this point in the history
…g-8360

Fix markdown tab render delay when toggling selected compared runs
  • Loading branch information
openshift-merge-bot[bot] authored Jul 5, 2024
2 parents aa75f48 + 9151654 commit f3339ac
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 137 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,10 @@ describe('Compare runs', () => {

const row = content.getRocCurveRowByName('wine-classification > metrics');
row.findRunName().should('contain.text', 'Run 1');
content.findRocCurveGraph().should('contain.text', 'Series #1');
content.findRocCurveGraph().should('include.text', '#1');

row.findRowCheckbox().uncheck();
content.findRocCurveGraph().should('not.contain.text', 'Series #1');
content.findRocCurveGraph().should('not.include.text', '#1');
});

it('displays ROC curve empty state when no artifacts are found', () => {
Expand All @@ -312,24 +312,6 @@ describe('Compare runs', () => {
content.findRocCruveSearchBar().type('invalid');
content.findRocCurveEmptyState().should('exist');
});

it('displays markdown fetched from S3 based on selected runs', () => {
cy.wait('@s3Loaded', {
timeout: 15000,
});

compareRunsMetricsContent.findMarkdownTab().click();
const markdownCompare = compareRunsMetricsContent
.findMarkdownTabContent()
.findMarkdownSelect(mockRun.run_id);

// check markdown content
markdownCompare.findArtifactContent().should('contain.text', 'This is a markdown file');

// check expanded graph
markdownCompare.findExpandButton().click();
compareRunsMetricsContent.findMarkdownTabContent().findExpandedMarkdown().should('exist');
});
});
});

Expand Down Expand Up @@ -397,32 +379,4 @@ const initIntercepts = () => {
);

initMlmdIntercepts(projectName);

cy.interceptOdh(
'GET /api/storage/:namespace',
{
path: {
namespace: projectName,
},
query: {
key: 'metrics-visualization-pipeline/16dbff18-a3d5-4684-90ac-4e6198a9da0f/markdown-visualization/markdown_artifact',
},
},
{ body: 'This is a markdown file' },
).as('s3Loaded');

cy.interceptOdh(
'GET /api/storage/:namespace/size',
{
path: {
namespace: projectName,
},
query: {
key: 'metrics-visualization-pipeline/16dbff18-a3d5-4684-90ac-4e6198a9da0f/markdown-visualization/markdown_artifact',
},
},
{
body: 100,
},
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,17 @@ import {
Stack,
StackItem,
} from '@patternfly/react-core';

import { PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes';

import { RunArtifact } from '~/concepts/pipelines/apiHooks/mlmd/types';
import { FullArtifactPath } from '~/concepts/pipelines/content/compareRuns/metricsSection/types';
import {
getFullArtifactPaths,
getFullArtifactPathLabel,
} from '~/concepts/pipelines/content/compareRuns/metricsSection/utils';
import { CompareRunsEmptyState } from '~/concepts/pipelines/content/compareRuns/CompareRunsEmptyState';
import { PipelineRunArtifactSelect } from '~/concepts/pipelines/content/compareRuns/metricsSection/PipelineRunArtifactSelect';
import MarkdownView from '~/components/MarkdownView';
import {
MAX_STORAGE_OBJECT_SIZE,
fetchStorageObject,
fetchStorageObjectSize,
} from '~/services/storageService';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import { extractS3UriComponents } from '~/concepts/pipelines/content/artifacts/utils';
import { MAX_STORAGE_OBJECT_SIZE } from '~/services/storageService';
import { bytesAsRoundedGiB } from '~/utilities/number';
import { PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes';

type MarkdownCompareProps = {
runArtifacts?: RunArtifact[];
configMap: Record<string, MarkdownAndTitle[]>;
runMap: Record<string, PipelineRunKFv2>;
isEmpty: boolean;
isLoaded: boolean;
};

Expand All @@ -45,60 +33,13 @@ export type MarkdownAndTitle = {
fileSize?: number;
};

const MarkdownCompare: React.FC<MarkdownCompareProps> = ({ runArtifacts, isLoaded }) => {
const MarkdownCompare: React.FC<MarkdownCompareProps> = ({
configMap,
runMap,
isLoaded,
isEmpty,
}) => {
const [expandedGraph, setExpandedGraph] = React.useState<MarkdownAndTitle | undefined>(undefined);
const { namespace } = usePipelinesAPI();

const fullArtifactPaths: FullArtifactPath[] = React.useMemo(() => {
if (!runArtifacts) {
return [];
}

return getFullArtifactPaths(runArtifacts);
}, [runArtifacts]);

const { configMap, runMap } = React.useMemo(() => {
const configMapBuilder: Record<string, MarkdownAndTitle[]> = {};
const runMapBuilder: Record<string, PipelineRunKFv2> = {};

fullArtifactPaths
.map((fullPath) => ({
run: fullPath.run,
title: getFullArtifactPathLabel(fullPath),
uri: fullPath.linkedArtifact.artifact.getUri(),
}))
.filter((markdown) => !!markdown.uri)
.forEach(async ({ uri, title, run }) => {
const uriComponents = extractS3UriComponents(uri);
if (!uriComponents) {
return;
}
const sizeBytes = await fetchStorageObjectSize(namespace, uriComponents.path).catch(
() => undefined,
);
const text = await fetchStorageObject(namespace, uriComponents.path).catch(() => null);

if (text === null) {
return;
}

runMapBuilder[run.run_id] = run;

const config = {
title,
config: text,
fileSize: sizeBytes,
};

if (run.run_id in configMapBuilder) {
configMapBuilder[run.run_id].push(config);
} else {
configMapBuilder[run.run_id] = [config];
}
});

return { configMap: configMapBuilder, runMap: runMapBuilder };
}, [fullArtifactPaths, namespace]);

if (!isLoaded) {
return (
Expand All @@ -108,9 +49,10 @@ const MarkdownCompare: React.FC<MarkdownCompareProps> = ({ runArtifacts, isLoade
);
}

if (!runArtifacts || runArtifacts.length === 0) {
if (isEmpty) {
return <CompareRunsEmptyState />;
}

if (Object.keys(configMap).length === 0) {
return (
<EmptyState variant={EmptyStateVariant.xs} data-testid="compare-runs-markdown-empty-state">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import React from 'react';
import { RunArtifact } from '~/concepts/pipelines/apiHooks/mlmd/types';
import { extractS3UriComponents } from '~/concepts/pipelines/content/artifacts/utils';
import { MarkdownAndTitle } from '~/concepts/pipelines/content/compareRuns/metricsSection/markdown/MarkdownCompare';
import {
getFullArtifactPathLabel,
getFullArtifactPaths,
} from '~/concepts/pipelines/content/compareRuns/metricsSection/utils';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import { PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes';
import { fetchStorageObject, fetchStorageObjectSize } from '~/services/storageService';
import { allSettledPromises } from '~/utilities/allSettledPromises';

const useFetchMarkdownMaps = (
markdownArtifacts?: RunArtifact[],
): {
configMap: Record<string, MarkdownAndTitle[]>;
runMap: Record<string, PipelineRunKFv2>;
configsLoaded: boolean;
} => {
const { namespace } = usePipelinesAPI();
const [configsLoaded, setConfigsLoaded] = React.useState(false);
const [configMapBuilder, setConfigMapBuilder] = React.useState<
Record<string, MarkdownAndTitle[]>
>({});
const [runMapBuilder, setRunMapBuilder] = React.useState<Record<string, PipelineRunKFv2>>({});

const fullArtifactPaths = React.useMemo(() => {
if (!markdownArtifacts) {
return [];
}

return getFullArtifactPaths(markdownArtifacts);
}, [markdownArtifacts]);

const fetchStorageObjectPromises = React.useMemo(
() =>
fullArtifactPaths
.filter((path) => !!path.linkedArtifact.artifact.getUri())
.map(async (path) => {
const { run } = path;
const uriComponents = extractS3UriComponents(path.linkedArtifact.artifact.getUri());
if (!uriComponents) {
return null;
}
const sizeBytes = await fetchStorageObjectSize(namespace, uriComponents.path).catch(
() => undefined,
);
const text = await fetchStorageObject(namespace, uriComponents.path).catch(() => null);

if (text === null) {
return null;
}

return { run, sizeBytes, text, path };
}),
[fullArtifactPaths, namespace],
);

React.useEffect(() => {
setConfigsLoaded(false);
setConfigMapBuilder({});
setRunMapBuilder({});

allSettledPromises(fetchStorageObjectPromises).then(([successes]) => {
successes.forEach((result) => {
if (result.value) {
const { text, sizeBytes, run, path } = result.value;
setRunMapBuilder((runMap) => ({ ...runMap, [run.run_id]: run }));

const config = {
title: getFullArtifactPathLabel(path),
config: text,
fileSize: sizeBytes,
};

setConfigMapBuilder((configMap) => ({
...configMap,
[run.run_id]: run.run_id in configMap ? [...configMap[run.run_id], config] : [config],
}));
}
});
setConfigsLoaded(true);
});
}, [fetchStorageObjectPromises]);

return { configMap: configMapBuilder, runMap: runMapBuilder, configsLoaded };
};

export default useFetchMarkdownMaps;
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';

import _ from 'lodash-es';
import { ExpandableSection, Tab, TabContentBody, TabTitleText, Tabs } from '@patternfly/react-core';
import { useCompareRuns } from '~/concepts/pipelines/content/compareRuns/CompareRunsContext';
import { useGetArtifactTypes } from '~/concepts/pipelines/apiHooks/mlmd/useGetArtifactTypes';
Expand All @@ -18,6 +18,7 @@ import RocCurveCompare from '~/concepts/pipelines/content/compareRuns/metricsSec
import ConfusionMatrixCompare from '~/concepts/pipelines/content/compareRuns/metricsSection/confusionMatrix/ConfusionMatrixCompare';
import MarkdownCompare from '~/concepts/pipelines/content/compareRuns/metricsSection/markdown/MarkdownCompare';
import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas';
import useFetchMarkdownMaps from '~/concepts/pipelines/content/compareRuns/metricsSection/markdown/useFetchMarkdownMaps';

export const CompareRunMetricsSection: React.FunctionComponent = () => {
const { runs, selectedRuns } = useCompareRuns();
Expand All @@ -29,39 +30,47 @@ export const CompareRunMetricsSection: React.FunctionComponent = () => {
);
const isS3EndpointAvailable = useIsAreaAvailable(SupportedArea.S3_ENDPOINT).status;

const selectedMlmdPackages = React.useMemo(
() =>
mlmdPackages.filter((mlmdPackage) =>
selectedRuns.some((run) => run.run_id === mlmdPackage.run.run_id),
),
[mlmdPackages, selectedRuns],
const runArtifacts: RunArtifact[] = React.useMemo(
() => getRunArtifacts(mlmdPackages),
[mlmdPackages],
);

const isLoaded = mlmdPackagesLoaded && artifactTypesLoaded;

const [
scalarMetricsArtifactData,
confusionMatrixArtifactData,
rocCurveArtifactData,
markdownArtifactData,
] = React.useMemo(() => {
const [markdownArtifacts, ...allArtifacts] = React.useMemo(() => {
if (!isLoaded) {
return [[], [], [], []];
}

const runArtifacts: RunArtifact[] = getRunArtifacts(selectedMlmdPackages);
return [
filterRunArtifactsByType(runArtifacts, artifactTypes, MetricsType.MARKDOWN),
filterRunArtifactsByType(runArtifacts, artifactTypes, MetricsType.SCALAR_METRICS),
filterRunArtifactsByType(runArtifacts, artifactTypes, MetricsType.CONFUSION_MATRIX),
filterRunArtifactsByType(runArtifacts, artifactTypes, MetricsType.ROC_CURVE),
filterRunArtifactsByType(runArtifacts, artifactTypes, MetricsType.MARKDOWN),
];
}, [artifactTypes, isLoaded, selectedMlmdPackages]);
}, [artifactTypes, isLoaded, runArtifacts]);

const { configMap, configsLoaded, runMap } = useFetchMarkdownMaps(markdownArtifacts);
const [selectedConfigMap, selectedRunMap] = React.useMemo(() => {
const selectedIds = selectedRuns.map((run) => run.run_id);
return [_.pick(configMap, selectedIds), _.pick(runMap, selectedIds)];
}, [configMap, runMap, selectedRuns]);

const filterSelected = React.useCallback(
(runArtifact: RunArtifact) => selectedRuns.some((run) => run.run_id === runArtifact.run.run_id),
[selectedRuns],
);

const [scalarMetricsArtifactData, confusionMatrixArtifactData, rocCurveArtifactData] =
React.useMemo(
() => allArtifacts.map((artifacts) => artifacts.filter(filterSelected)),
[allArtifacts, filterSelected],
);

return (
<ExpandableSection
toggleText="Metrics"
onToggle={(_, isOpen) => setIsSectionOpen(isOpen)}
onToggle={(_event, isOpen) => setIsSectionOpen(isOpen)}
isExpanded={isSectionOpen}
isIndented
data-testid="compare-runs-metrics-content"
Expand Down Expand Up @@ -104,7 +113,12 @@ export const CompareRunMetricsSection: React.FunctionComponent = () => {
data-testid="compare-runs-markdown-tab"
>
<TabContentBody hasPadding data-testid="compare-runs-markdown-tab-content">
<MarkdownCompare runArtifacts={markdownArtifactData} isLoaded={isLoaded} />
<MarkdownCompare
configMap={selectedConfigMap}
runMap={selectedRunMap}
isEmpty={selectedRuns.length === 0}
isLoaded={isLoaded && configsLoaded}
/>
</TabContentBody>
</Tab>
)}
Expand Down

0 comments on commit f3339ac

Please sign in to comment.