Skip to content

Commit 244666c

Browse files
Philip-CarneiroPhilip-Carneiro
authored
truncate error msg from transfer jobs (kubeflow#2625)
* truncate error msg from transfer jobs Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com> * add changes requested and updated tests Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com> --------- Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com>
1 parent c6ee0da commit 244666c

3 files changed

Lines changed: 196 additions & 13 deletions

File tree

‎clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/modelTransferJobs.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,18 @@ class ModelTransferJobsTableRow extends TableRow {
44
findJobName() {
55
return this.find().findByTestId('job-name');
66
}
7+
8+
findStatus() {
9+
return this.find().findByTestId('job-status');
10+
}
11+
12+
findErrorMessage() {
13+
return this.find().findByTestId('job-error-message');
14+
}
15+
16+
findRetryButton() {
17+
return this.find().findByTestId('job-retry-button');
18+
}
719
}
820

921
class ModelTransferJobsPage {
@@ -57,6 +69,14 @@ class ModelTransferJobsPage {
5769
findEmptyState() {
5870
return cy.findByTestId('empty-model-transfer-jobs');
5971
}
72+
73+
findRetryModal() {
74+
return cy.findByTestId('retry-job-modal');
75+
}
76+
77+
findRetryModalSubmitButton() {
78+
return this.findRetryModal().findByTestId('retry-job-submit-button');
79+
}
6080
}
6181

6282
export const modelTransferJobsPage = new ModelTransferJobsPage();

‎clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelTransferJobs.cy.ts‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,146 @@ describe('Model transfer jobs', () => {
477477
});
478478
});
479479

480+
it('should show truncated error message below Failed label when errorMessage exists', () => {
481+
interceptWithSingleFailedJob(
482+
'job-with-error',
483+
'Connection timeout while uploading to destination bucket',
484+
);
485+
486+
modelTransferJobsPage.visit(modelRegistryName);
487+
modelTransferJobsPage.findTableRows().should('have.length', 1);
488+
489+
const row = modelTransferJobsPage.getRow('job-with-error');
490+
row.findStatus().should('contain.text', 'Failed');
491+
row
492+
.findErrorMessage()
493+
.should('be.visible')
494+
.and('contain.text', 'Connection timeout while uploading to destination bucket');
495+
});
496+
497+
it('should not show error message when job is Failed but errorMessage is undefined', () => {
498+
const failedJobList = mockModelTransferJobList({
499+
items: [
500+
mockModelTransferJob({
501+
id: 'job-failed-no-msg',
502+
name: 'job-failed-no-msg',
503+
jobDisplayName: 'job-failed-no-msg',
504+
status: ModelTransferJobStatus.FAILED,
505+
errorMessage: undefined,
506+
}),
507+
],
508+
size: 1,
509+
pageSize: 10,
510+
nextPageToken: '',
511+
});
512+
513+
cy.intercept(
514+
'GET',
515+
`**/api/${MODEL_REGISTRY_API_VERSION}/model_registry/${modelRegistryName}/model_transfer_jobs*`,
516+
mockModArchResponse(failedJobList),
517+
);
518+
519+
modelTransferJobsPage.visit(modelRegistryName);
520+
521+
const row = modelTransferJobsPage.getRow('job-failed-no-msg');
522+
row.findStatus().should('contain.text', 'Failed');
523+
row.findErrorMessage().should('not.exist');
524+
});
525+
526+
it('should not show error message when errorMessage is an empty string', () => {
527+
const failedJobList = mockModelTransferJobList({
528+
items: [
529+
mockModelTransferJob({
530+
id: 'job-failed-empty-msg',
531+
name: 'job-failed-empty-msg',
532+
jobDisplayName: 'job-failed-empty-msg',
533+
status: ModelTransferJobStatus.FAILED,
534+
errorMessage: '',
535+
}),
536+
],
537+
size: 1,
538+
pageSize: 10,
539+
nextPageToken: '',
540+
});
541+
542+
cy.intercept(
543+
'GET',
544+
`**/api/${MODEL_REGISTRY_API_VERSION}/model_registry/${modelRegistryName}/model_transfer_jobs*`,
545+
mockModArchResponse(failedJobList),
546+
);
547+
548+
modelTransferJobsPage.visit(modelRegistryName);
549+
550+
const row = modelTransferJobsPage.getRow('job-failed-empty-msg');
551+
row.findStatus().should('contain.text', 'Failed');
552+
row.findErrorMessage().should('not.exist');
553+
});
554+
555+
it('should open status modal when clicking the truncated error message', () => {
556+
interceptWithSingleFailedJob('job-click-error', 'Upload failed: insufficient storage');
557+
558+
cy.intercept('GET', '**/model_transfer_jobs/job-click-error/events*', {
559+
forceNetworkError: true,
560+
});
561+
562+
modelTransferJobsPage.visit(modelRegistryName);
563+
564+
const row = modelTransferJobsPage.getRow('job-click-error');
565+
row.findErrorMessage().click();
566+
567+
cy.findByTestId('transfer-job-status-modal').should('be.visible');
568+
cy.findByLabelText('Close').click();
569+
cy.findByTestId('transfer-job-status-modal').should('not.exist');
570+
});
571+
572+
it('should show retry button for failed jobs and open retry modal on click', () => {
573+
interceptWithSingleFailedJob('job-retry-test', 'Connection error');
574+
575+
cy.intercept(
576+
'PUT',
577+
`**/api/${MODEL_REGISTRY_API_VERSION}/model_registry/${modelRegistryName}/model_transfer_jobs/job-retry-test*`,
578+
{ statusCode: 200, body: {} },
579+
).as('retryJob');
580+
581+
modelTransferJobsPage.visit(modelRegistryName);
582+
583+
const row = modelTransferJobsPage.getRow('job-retry-test');
584+
row.findRetryButton().should('be.visible').click();
585+
586+
modelTransferJobsPage.findRetryModal().should('be.visible');
587+
modelTransferJobsPage.findRetryModal().contains('Retry model transfer job?').should('exist');
588+
modelTransferJobsPage.findRetryModalSubmitButton().should('be.visible');
589+
});
590+
591+
it('should not show error message for non-FAILED statuses', () => {
592+
const completedJobList = mockModelTransferJobList({
593+
items: [
594+
mockModelTransferJob({
595+
id: 'job-completed-no-error',
596+
name: 'job-completed-no-error',
597+
jobDisplayName: 'job-completed-no-error',
598+
status: ModelTransferJobStatus.COMPLETED,
599+
}),
600+
],
601+
size: 1,
602+
pageSize: 10,
603+
nextPageToken: '',
604+
});
605+
606+
cy.intercept(
607+
'GET',
608+
`**/api/${MODEL_REGISTRY_API_VERSION}/model_registry/${modelRegistryName}/model_transfer_jobs*`,
609+
mockModArchResponse(completedJobList),
610+
);
611+
612+
modelTransferJobsPage.visit(modelRegistryName);
613+
614+
const row = modelTransferJobsPage.getRow('job-completed-no-error');
615+
row.findStatus().should('contain.text', 'Complete');
616+
row.findErrorMessage().should('not.exist');
617+
row.findRetryButton().should('not.exist');
618+
});
619+
480620
it('should show the empty state when there are no transfer jobs', () => {
481621
const emptyList = mockModelTransferJobList({
482622
items: [],

‎clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelTransferJobs/ModelTransferJobTableRow.tsx‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -130,27 +130,50 @@ const ModelTransferJobTableRow: React.FC<ModelTransferJobTableRowProps> = ({
130130
</Content>
131131
</Td>
132132
<Td dataLabel="Transfer job status">
133-
<Flex alignItems={{ default: 'alignItemsCenter' }} spaceItems={{ default: 'spaceItemsSm' }}>
133+
<Flex direction={{ default: 'column' }} spaceItems={{ default: 'spaceItemsXs' }}>
134134
<FlexItem>
135-
<Label
136-
color={statusInfo.color}
137-
icon={statusInfo.icon}
138-
data-testid="job-status"
139-
isClickable
140-
onClick={() => setIsStatusModalOpen(true)}
135+
<Flex
136+
alignItems={{ default: 'alignItemsCenter' }}
137+
spaceItems={{ default: 'spaceItemsSm' }}
141138
>
142-
{statusInfo.label}
143-
</Label>
139+
<FlexItem>
140+
<Label
141+
color={statusInfo.color}
142+
icon={statusInfo.icon}
143+
data-testid="job-status"
144+
isClickable
145+
onClick={() => setIsStatusModalOpen(true)}
146+
>
147+
{statusInfo.label}
148+
</Label>
149+
</FlexItem>
150+
{job.status === ModelTransferJobStatus.FAILED && onRequestRetry && (
151+
<FlexItem>
152+
<Button
153+
variant="link"
154+
isInline
155+
onClick={() => onRequestRetry(job)}
156+
data-testid="job-retry-button"
157+
>
158+
Retry
159+
</Button>
160+
</FlexItem>
161+
)}
162+
</Flex>
144163
</FlexItem>
145-
{job.status === ModelTransferJobStatus.FAILED && onRequestRetry && (
164+
{job.status === ModelTransferJobStatus.FAILED && job.errorMessage && (
146165
<FlexItem>
147166
<Button
148167
variant="link"
149168
isInline
150-
onClick={() => onRequestRetry(job)}
151-
data-testid="job-retry-button"
169+
onClick={() => setIsStatusModalOpen(true)}
170+
data-testid="job-error-message"
171+
style={{
172+
textDecoration: 'underline dotted',
173+
textUnderlineOffset: 'var(--pf-t--global--spacer--xs)',
174+
}}
152175
>
153-
Retry
176+
<Truncate content={job.errorMessage} />
154177
</Button>
155178
</FlexItem>
156179
)}

0 commit comments

Comments
 (0)