Skip to content
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.

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

allow to restore archived runs whose associate experiments are archived #3757

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,40 @@ class PipelineRunsTable {
return cy.interceptOdh(
'POST /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/runs/:runId',
{ path: { namespace, serviceName: 'dspa', runId: `${runId}:unarchive` } },
(req) => {
req.reply({
body: {},
});
},
);
}

mockRestoreRunFails(runId: string, namespace: string) {
return cy.interceptOdh(
'POST /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/runs/:runId',
{ path: { namespace, serviceName: 'dspa', runId: `${runId}:unarchive` } },
(req) => {
req.reply({
error: 'Failed to unarchive a run: FailedPreconditionError',
code: 9,
message: 'Failed to unarchive a run: FailedPreconditionError',
details: [
{
'@type': 'type.googleapis.com/google.rpc.Status',
code: 9,
message:
'Failed to unarchive run c4b8745e-fedb-4127-aee3-77deb54cc7b5 as experiment b83e522a-14a7-4fb0-8399-64a620f0b9c7 must be un-archived first',
},
],
});
},
);
}

mockRestoreExperiment(experimentId: string, namespace: string) {
return cy.interceptOdh(
'POST /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/experiments/:experimentId',
{ path: { namespace, serviceName: 'dspa', experimentId: `${experimentId}:unarchive` } },
(req) => {
req.reply({ body: {} });
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,60 @@ import { Modal } from '~/__tests__/cypress/cypress/pages/components/Modal';
class RestoreModal extends Modal {
protected testId;

constructor(title: string, testId: string) {
protected submitName;

constructor(title: string, testId: string, name?: string) {
super(title);
this.testId = testId;
this.submitName = name;
}

find() {
return cy.findByTestId(this.testId).parents('div[role="dialog"]');
}

findAlertMessage() {
return cy.findByTestId('single-restoring-alert-message');
}

findSubmitButton() {
return this.findFooter().findByRole('button', { name: 'Restore', hidden: true });
return this.findFooter().findByRole('button', {
name: this.submitName || 'Restore',
hidden: true,
});
}

findCancelButton() {
return this.findFooter().findByRole('button', {
name: 'Cancel',
hidden: true,
});
}

findRetryButton() {
return this.findFooter().findByRole('button', {
name: 'Retry',
hidden: true,
});
}

findErrorMessage() {
return this.find().findByTestId('error-message-alert');
}
}

export const restoreRunModal = new RestoreModal('Restore run?', 'restore-run-modal');
export const bulkRestoreRunModal = new RestoreModal('Restore runs?', 'restore-run-modal');
export const bulkRestoreRunWithArchivedExperimentModal = new RestoreModal(
'Restore runs?',
'restore-run-modal',
'Restore runs and experiments',
);
export const restoreRunWithArchivedExperimentModal = new RestoreModal(
'Restore runs?',
'restore-run-modal',
'Restore run and experiment',
);
export const restoreExperimentModal = new RestoreModal(
'Restore experiment?',
'restore-experiment-modal',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import {
bulkArchiveRunModal,
duplicateRunPage,
duplicateSchedulePage,
bulkRestoreRunWithArchivedExperimentModal,
restoreRunWithArchivedExperimentModal,
} from '~/__tests__/cypress/cypress/pages/pipelines';
import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url';
import { be } from '~/__tests__/cypress/cypress/utils/should';
Expand Down Expand Up @@ -91,14 +93,43 @@ const mockArchivedRuns = [
pipeline_id: pipelineId,
pipeline_version_id: 'test-version-2',
},
experiment_id: 'test-experiment-3',
created_at: '2024-02-20T00:00:00Z',
state: RuntimeStateKF.SUCCEEDED,
}),
];

const mockArchivedRunsWithArchivedExperiments = [
buildMockRunKF({
display_name: 'experiment archived run 1',
run_id: 'run-1',
pipeline_version_reference: {
pipeline_id: pipelineId,
pipeline_version_id: 'test-version-1',
},
experiment_id: 'test-experiment-1',
created_at: '2024-02-05T00:00:00Z',
state: RuntimeStateKF.SUCCEEDED,
}),
buildMockRunKF({
display_name: 'experiment archived run 2',
run_id: 'run-2',
pipeline_version_reference: {
pipeline_id: pipelineId,
pipeline_version_id: 'test-version-2',
},
experiment_id: 'test-experiment-2',
created_at: '2024-02-20T00:00:00Z',
state: RuntimeStateKF.SUCCEEDED,
}),
];

const mockExperimentIds = [
...new Set([...mockActiveRuns, ...mockArchivedRuns].map((mockRun) => mockRun.experiment_id)),
...new Set(
[...mockActiveRuns, ...mockArchivedRuns, ...mockArchivedRunsWithArchivedExperiments].map(
(mockRun) => mockRun.experiment_id,
),
),
];
const mockVersionIds = [
...new Set(
Expand Down Expand Up @@ -757,6 +788,76 @@ describe('Pipeline runs', () => {
});
});
});

it('restore multiple runs with archived experiments', () => {
archivedRunsTable.mockGetArchivedRuns(
[...mockArchivedRuns, ...mockArchivedRunsWithArchivedExperiments],
projectName,
);
pipelineRunsGlobal.visit(projectName, 'archived');
[...mockArchivedRuns, ...mockArchivedRunsWithArchivedExperiments].forEach((archivedRun) => {
archivedRunsTable.mockRestoreRun(archivedRun.run_id, projectName);
if (archivedRun.experiment_id === 'test-experiment-2') {
archivedRunsTable.mockRestoreExperiment(archivedRun.experiment_id, projectName);
}
archivedRunsTable.getRowByName(archivedRun.display_name).findCheckbox().click();
});
pipelineRunsGlobal.findRestoreRunButton().click();
archivedRunsTable.mockGetRuns(
[...mockArchivedRuns, ...mockArchivedRunsWithArchivedExperiments],
[],
projectName,
);
bulkRestoreRunWithArchivedExperimentModal.findSubmitButton().click();
archivedRunsTable.findEmptyState().should('exist');
});

it('handle error when restore multiple runs with archived experiments fails', () => {
archivedRunsTable.mockGetArchivedRuns(
[...mockArchivedRuns, ...mockArchivedRunsWithArchivedExperiments],
projectName,
);
pipelineRunsGlobal.visit(projectName, 'archived');
[...mockArchivedRuns, ...mockArchivedRunsWithArchivedExperiments].forEach((archivedRun) => {
archivedRunsTable.mockRestoreRunFails(archivedRun.run_id, projectName);
if (archivedRun.experiment_id === 'test-experiment-2') {
archivedRunsTable.mockRestoreExperiment(archivedRun.experiment_id, projectName);
}
archivedRunsTable.getRowByName(archivedRun.display_name).findCheckbox().click();
});
pipelineRunsGlobal.findRestoreRunButton().click();
archivedRunsTable.mockGetRuns(
[...mockArchivedRuns, ...mockArchivedRunsWithArchivedExperiments],
[],
projectName,
);
bulkRestoreRunWithArchivedExperimentModal.findSubmitButton().click();
bulkRestoreRunWithArchivedExperimentModal.findErrorMessage().should('exist');

[...mockArchivedRuns, ...mockArchivedRunsWithArchivedExperiments].forEach((archivedRun) => {
archivedRunsTable.mockRestoreRun(archivedRun.run_id, projectName);
});
//retry
bulkRestoreRunWithArchivedExperimentModal.findRetryButton().click();
archivedRunsTable.findEmptyState().should('exist');
});

it('restore a single run', () => {
archivedRunsTable.mockGetArchivedRuns(
[...mockArchivedRuns, ...mockArchivedRunsWithArchivedExperiments],
projectName,
);
pipelineRunsGlobal.visit(projectName, 'archived');
const [, runToRestore] = mockArchivedRunsWithArchivedExperiments;

archivedRunsTable.mockRestoreRun(runToRestore.run_id, projectName);
archivedRunsTable.getRowByName(runToRestore.display_name).findKebabAction('Restore').click();

archivedRunsTable.mockGetRuns([runToRestore], [mockArchivedRuns[1]], projectName);
restoreRunWithArchivedExperimentModal.findAlertMessage().should('exist');
restoreRunWithArchivedExperimentModal.findSubmitButton().click();
archivedRunsTable.shouldRowNotExist(runToRestore.display_name);
});
});

describe('Schedules', () => {
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/concepts/dashboard/DashboardModalFooter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type DashboardModalFooterProps = {
isSubmitLoading?: boolean;
isCancelDisabled?: boolean;
alertTitle?: string;
error?: Error;
error?: Error | React.ReactNode;
alertLinks?: React.ReactNode;
};

Expand All @@ -46,7 +46,7 @@ const DashboardModalFooter: React.FC<DashboardModalFooterProps> = ({
title={alertTitle}
actionLinks={alertLinks}
>
{error.message}
{error instanceof Error ? error.message : error}
</Alert>
</StackItem>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { ColumnsIcon } from '@patternfly/react-icons';

import { TableBase, getTableColumnSort, useCheckboxTable } from '~/components/table';
import { PipelineRunKF } from '~/concepts/pipelines/kfTypes';
import { ExperimentKF, PipelineRunKF, StorageStateKF } from '~/concepts/pipelines/kfTypes';
import { getPipelineRunColumns } from '~/concepts/pipelines/content/tables/columns';
import PipelineRunTableRow from '~/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableRow';
import DashboardEmptyTableView from '~/concepts/dashboard/DashboardEmptyTableView';
Expand All @@ -24,6 +24,8 @@
ExperimentContext,
useContextExperimentArchivedOrDeleted,
} from '~/pages/pipelines/global/experiments/ExperimentContext';
import { PipelineRunExperimentsContext } from '~/pages/pipelines/global/runs/PipelineRunExperimentsContext';
import RestoreRunWithArchivedExperimentModal from '~/pages/pipelines/global/runs/RestoreRunWithArchivedExperimentModal';
import { CustomMetricsColumnsModal } from './CustomMetricsColumnsModal';
import { UnavailableMetricValue } from './UnavailableMetricValue';
import { useMetricColumns } from './useMetricColumns';
Expand Down Expand Up @@ -58,6 +60,7 @@
}) => {
const navigate = useNavigate();
const { experiment } = React.useContext(ExperimentContext);
const { experiments: allExperiments } = React.useContext(PipelineRunExperimentsContext);
const { namespace, refreshAllAPI } = usePipelinesAPI();
const { onClearFilters, ...filterToolbarProps } = usePipelineFilterSearchParams(setFilter);
const { metricsColumnNames, runs, runArtifactsError, runArtifactsLoaded, metricsNames } =
Expand All @@ -82,14 +85,35 @@

return acc;
}, []);

const restoreButtonTooltipRef = React.useRef(null);
const { isExperimentArchived } = useContextExperimentArchivedOrDeleted();
const archivedExperiments = selectedRuns.reduce<ExperimentKF[]>((acc, selectedRun) => {
const currentExperiment = allExperiments.find(
(e) => e.experiment_id === selectedRun.experiment_id,
);

if (currentExperiment && currentExperiment.storage_state === StorageStateKF.ARCHIVED) {
// Create a Set to track unique experiment_id
if (
!acc.some(
(selectedExperiment) =>

Check warning on line 99 in frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTable.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTable.tsx#L99

Added line #L99 was not covered by tests
selectedExperiment.experiment_id === currentExperiment.experiment_id,
)
) {
acc.push(currentExperiment);
}
}

return acc;
}, []);

const { isExperimentArchived: isContextExperimentArchived } =
useContextExperimentArchivedOrDeleted();
const primaryToolbarAction = React.useMemo(() => {
if (runType === PipelineRunType.ARCHIVED) {
return (
<>
{isExperimentArchived && (
{isContextExperimentArchived && (
<Tooltip
content="Archived runs cannot be restored until its associated experiment is restored."
triggerRef={restoreButtonTooltipRef}
Expand All @@ -99,7 +123,7 @@
data-testid="restore-button"
variant="primary"
isDisabled={!selectedIds.length}
isAriaDisabled={isExperimentArchived}
isAriaDisabled={isContextExperimentArchived}
onClick={() => setIsRestoreModalOpen(true)}
ref={restoreButtonTooltipRef}
>
Expand All @@ -121,15 +145,15 @@
);
}, [
runType,
isExperimentArchived,
isContextExperimentArchived,
selectedIds.length,
navigate,
namespace,
experiment?.experiment_id,
]);

const compareRunsAction =
!isExperimentArchived && runType === PipelineRunType.ACTIVE ? (
!isContextExperimentArchived && runType === PipelineRunType.ACTIVE ? (
<Tooltip content="Select up to 10 runs to compare.">
<Button
key="compare-runs"
Expand Down Expand Up @@ -273,15 +297,28 @@
}}
/>
)}
{isRestoreModalOpen && (
<RestoreRunModal
runs={selectedRuns}
onCancel={() => {
setIsRestoreModalOpen(false);
setSelectedIds([]);
}}
/>
)}
{isRestoreModalOpen &&
(!archivedExperiments.length ? (
<RestoreRunModal
runs={selectedRuns}
onCancel={() => {
setIsRestoreModalOpen(false);
setSelectedIds([]);
}}
/>
) : (
<RestoreRunWithArchivedExperimentModal
selectedRuns={selectedRuns}
archivedExperiments={archivedExperiments}
onClose={(restored: boolean) => {
if (restored) {
refreshAllAPI();
}
setIsRestoreModalOpen(false);
setSelectedIds([]);
}}
/>
))}
{isDeleteModalOpen && (
<DeletePipelineRunsModal
toDeleteResources={selectedRuns}
Expand Down
Loading