Skip to content

Commit 6513dd6

Browse files
zonia3000zonia3000
authored
Merge pull request #415 from fractal-analytics-platform/various-improvements
Jobs improvements
2 parents e184c0f + fd8b5af commit 6513dd6

File tree

13 files changed

+196
-58
lines changed

13 files changed

+196
-58
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
# Unreleased
44

5+
* Improved visualization of details on job logs modal (\#415).
6+
* Added job id filter on admin jobs page (\#415).
57
* Added spinner on workflow task modal when tasks list is loading (\#410).
68
* Implemented import and export of workflow task arguments (\#410).
79
* Improved sorting of users in dropdown of the admin jobs page (\#402).

‎__tests__/JobLogsModal.test.js‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { describe, it, expect, vi } from 'vitest';
2+
import { fireEvent, render } from '@testing-library/svelte';
3+
4+
class MockModal {
5+
show = vi.fn();
6+
}
7+
MockModal.getInstance = vi.fn();
8+
9+
global.window.bootstrap = {
10+
Modal: MockModal
11+
};
12+
13+
import JobLogsModal from '../src/lib/components/jobs/JobLogsModal.svelte';
14+
15+
describe('JobLogsModal', async () => {
16+
it('display error log fully highlighted', async () => {
17+
const result = render(JobLogsModal);
18+
const error = `TASK ERROR:Task id: 20 (Create OME-Zarr structure), e.workflow_task_order=0
19+
TRACEBACK:
20+
Command "/tmp/FRACTAL_TASKS_DIR/.fractal/fractal-tasks-core0.14.1/venv/bin/python" is not valid. Hint: make sure that it is executable.`;
21+
await result.component.show({ status: 'failed', log: error });
22+
const pre = result.container.querySelector('pre');
23+
expect(pre.classList.contains('highlight')).eq(true);
24+
expect(pre.innerHTML).eq(error);
25+
});
26+
27+
it('display log with highlighting and hidden details', async () => {
28+
const result = render(JobLogsModal);
29+
const error = `TASK ERROR:Task id: 15 (Create OME-Zarr structure), e.workflow_task_order=0
30+
TRACEBACK:
31+
2024-01-29 16:52:02,328; INFO; START create_ome_zarr task
32+
Traceback (most recent call last):
33+
File "/tmp/FRACTAL_TASKS_DIR/.fractal/fractal-tasks-core0.14.1/venv/lib/python3.10/site-packages/fractal_tasks_core/tasks/create_ome_zarr.py", line 470, in <module>
34+
run_fractal_task(
35+
File "/tmp/FRACTAL_TASKS_DIR/.fractal/fractal-tasks-core0.14.1/venv/lib/python3.10/site-packages/fractal_tasks_core/tasks/_utils.py", line 79, in run_fractal_task
36+
metadata_update = task_function(**pars)
37+
File "pydantic/decorator.py", line 40, in pydantic.decorator.validate_arguments.validate.wrapper_function
38+
File "pydantic/main.py", line 341, in pydantic.main.BaseModel.__init__
39+
pydantic.error_wrappers.ValidationError: 1 validation error for CreateOmeZarr
40+
allowed_channels
41+
field required (type=value_error.missing)`;
42+
await result.component.show({ status: 'failed', log: error });
43+
const pre = result.container.querySelector('pre');
44+
let divs = pre.querySelectorAll('div');
45+
expect(divs.length).eq(2);
46+
expect(divs[0].classList.contains('highlight')).eq(true);
47+
expect(divs[0].innerHTML).eq(
48+
'TASK ERROR:Task id: 15 (Create OME-Zarr structure), e.workflow_task_order=0\n'
49+
);
50+
expect(divs[1].classList.contains('highlight')).eq(true);
51+
expect(divs[1].innerHTML)
52+
.eq(`pydantic.error_wrappers.ValidationError: 1 validation error for CreateOmeZarr
53+
allowed_channels
54+
field required (type=value_error.missing)\n`);
55+
expect(pre.querySelectorAll('button').length).eq(1);
56+
await fireEvent.click(result.getByRole('button', { name: /details hidden/ }));
57+
divs = pre.querySelectorAll('div');
58+
expect(divs.length).eq(3);
59+
expect(divs[0].classList.contains('highlight')).eq(true);
60+
expect(divs[1].classList.contains('highlight')).eq(false);
61+
expect(divs[2].classList.contains('highlight')).eq(true);
62+
expect(divs[1].innerHTML.startsWith('TRACEBACK')).eq(true);
63+
expect(pre.querySelectorAll('button').length).eq(0);
64+
});
65+
66+
it('display successful log', async () => {
67+
const result = render(JobLogsModal);
68+
const log = 'Successful log...';
69+
await result.component.show({ status: 'done', log });
70+
const pre = result.container.querySelector('pre');
71+
expect(pre.classList.contains('highlight')).eq(false);
72+
expect(pre.innerHTML).eq(log);
73+
});
74+
});

‎__tests__/JobsList.test.js‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -54,61 +54,61 @@ describe('JobsList', () => {
5454
await fireEvent.change(projectFilter, { target: { value: '1' } });
5555
table = result.getByRole('table');
5656
expect(table.querySelectorAll('tbody tr').length).eq(1);
57-
expect(table.querySelectorAll('tbody tr td')[6].textContent).eq('input1');
57+
expect(table.querySelectorAll('tbody tr td')[7].textContent).eq('input1');
5858
await clearFilters(result);
5959

6060
// Filter by workflow
6161
await fireEvent.change(workflowFilter, { target: { value: '2' } });
6262
table = result.getByRole('table');
6363
expect(table.querySelectorAll('tbody tr').length).eq(2);
64-
expect(table.querySelectorAll('tbody tr:nth-child(1) td')[6].textContent).eq('input3');
65-
expect(table.querySelectorAll('tbody tr:nth-child(2) td')[6].textContent).eq('input2');
64+
expect(table.querySelectorAll('tbody tr:nth-child(1) td')[7].textContent).eq('input3');
65+
expect(table.querySelectorAll('tbody tr:nth-child(2) td')[7].textContent).eq('input2');
6666
await clearFilters(result);
6767

6868
// Filter by input dataset
6969
await fireEvent.change(inputDatasetFilter, { target: { value: '3' } });
7070
table = result.getByRole('table');
7171
expect(table.querySelectorAll('tbody tr').length).eq(1);
72-
expect(table.querySelectorAll('tbody tr td')[6].textContent).eq('input2');
72+
expect(table.querySelectorAll('tbody tr td')[7].textContent).eq('input2');
7373
await clearFilters(result);
7474

7575
// Filter by output dataset
7676
await fireEvent.change(outputDatasetFilter, { target: { value: '4' } });
7777
table = result.getByRole('table');
7878
expect(table.querySelectorAll('tbody tr').length).eq(1);
79-
expect(table.querySelectorAll('tbody tr td')[7].textContent).eq('output2');
79+
expect(table.querySelectorAll('tbody tr td')[8].textContent).eq('output2');
8080
await clearFilters(result);
8181

8282
// Filter by job status
8383
await fireEvent.change(statusFilter, { target: { value: 'submitted' } });
8484
table = result.getByRole('table');
8585
expect(table.querySelectorAll('tbody tr').length).eq(1);
86-
expect(table.querySelectorAll('tbody tr td')[6].textContent).eq('input3');
86+
expect(table.querySelectorAll('tbody tr td')[7].textContent).eq('input3');
8787
await clearFilters(result);
8888

8989
// Verify default sorting
9090
table = result.getByRole('table');
91-
expect(table.querySelectorAll('tbody tr:nth-child(1) td')[2].textContent).eq(
91+
expect(table.querySelectorAll('tbody tr:nth-child(1) td')[3].textContent).eq(
9292
'10/30/2023, 10:30:38 AM'
9393
);
94-
expect(table.querySelectorAll('tbody tr:nth-child(2) td')[2].textContent).eq(
94+
expect(table.querySelectorAll('tbody tr:nth-child(2) td')[3].textContent).eq(
9595
'10/30/2023, 10:15:38 AM'
9696
);
97-
expect(table.querySelectorAll('tbody tr:nth-child(3) td')[2].textContent).eq(
97+
expect(table.querySelectorAll('tbody tr:nth-child(3) td')[3].textContent).eq(
9898
'10/30/2023, 10:00:38 AM'
9999
);
100100

101101
// Sort by start date
102-
const startDateSorter = table.querySelector('thead th:nth-child(3)');
102+
const startDateSorter = table.querySelector('thead th:nth-child(4)');
103103
await fireEvent.click(startDateSorter);
104104
table = result.getByRole('table');
105-
expect(table.querySelectorAll('tbody tr:nth-child(1) td')[2].textContent).eq(
105+
expect(table.querySelectorAll('tbody tr:nth-child(1) td')[3].textContent).eq(
106106
'10/30/2023, 10:00:38 AM'
107107
);
108-
expect(table.querySelectorAll('tbody tr:nth-child(2) td')[2].textContent).eq(
108+
expect(table.querySelectorAll('tbody tr:nth-child(2) td')[3].textContent).eq(
109109
'10/30/2023, 10:15:38 AM'
110110
);
111-
expect(table.querySelectorAll('tbody tr:nth-child(3) td')[2].textContent).eq(
111+
expect(table.querySelectorAll('tbody tr:nth-child(3) td')[3].textContent).eq(
112112
'10/30/2023, 10:30:38 AM'
113113
);
114114
});
@@ -169,19 +169,19 @@ describe('JobsList', () => {
169169
});
170170
let table = result.getByRole('table');
171171
expect(table.querySelectorAll('tbody tr').length).eq(3);
172-
expect(table.querySelectorAll('tbody tr:nth-child(1) td')[0].textContent).contain('submitted');
173-
expect(table.querySelectorAll('tbody tr:nth-child(2) td')[0].textContent).contain('failed');
174-
expect(table.querySelectorAll('tbody tr:nth-child(3) td')[0].textContent).contain('done');
172+
expect(table.querySelectorAll('tbody tr:nth-child(1) td')[1].textContent).contain('submitted');
173+
expect(table.querySelectorAll('tbody tr:nth-child(2) td')[1].textContent).contain('failed');
174+
expect(table.querySelectorAll('tbody tr:nth-child(3) td')[1].textContent).contain('done');
175175

176176
vi.advanceTimersByTime(3500);
177177
vi.useRealTimers();
178178
// trigger table update
179179
await new Promise(setTimeout);
180180

181181
table = result.getByRole('table');
182-
expect(table.querySelectorAll('tbody tr:nth-child(1) td')[0].textContent).contain('done');
183-
expect(table.querySelectorAll('tbody tr:nth-child(2) td')[0].textContent).contain('failed');
184-
expect(table.querySelectorAll('tbody tr:nth-child(3) td')[0].textContent).contain('done');
182+
expect(table.querySelectorAll('tbody tr:nth-child(1) td')[1].textContent).contain('done');
183+
expect(table.querySelectorAll('tbody tr:nth-child(2) td')[1].textContent).contain('failed');
184+
expect(table.querySelectorAll('tbody tr:nth-child(3) td')[1].textContent).contain('done');
185185
} finally {
186186
vi.useRealTimers();
187187
}

‎__tests__/job_utilities.test.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,16 @@ it('detect parts of a workflow task error message with short traceback', () => {
5959
expect(parts[2].highlight).eq(true);
6060
});
6161

62+
it('detect parts of a workflow task error message with short traceback, ignoring uppercase traceback', () => {
63+
const parts = extractJobErrorParts(shortTracebackError, true);
64+
expect(parts.length).eq(1);
65+
expect(parts[0].text)
66+
.eq(`TASK ERROR:Task id: 20 (Create OME-Zarr structure), e.workflow_task_order=0
67+
TRACEBACK:
68+
Command "/tmp/FRACTAL_TASKS_DIR/.fractal/fractal-tasks-core0.14.1/venv/bin/python" is not valid. Hint: make sure that it is executable.`);
69+
expect(parts[0].highlight).eq(false);
70+
});
71+
6272
it('detect parts of a workflow task error message without traceback', () => {
6373
const parts = extractJobErrorParts('foo');
6474
expect(parts.length).eq(1);

‎src/lib/common/job_utilities.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
1+
const completeTracebackLine = 'Traceback (most recent call last):';
2+
13
/**
24
* Split the error of a failed workflow job into multiple parts, marking the relevents ones,
35
* so that they can be extracted or highlighted in a different way in the UI.
46
*
57
* @param {string|null} log
8+
* @param {boolean} ignoreUppercaseTraceback
69
* @returns {Array<{text: string, highlight: boolean}>}
710
*/
8-
export function extractJobErrorParts(log) {
11+
export function extractJobErrorParts(log, ignoreUppercaseTraceback = false) {
912
if (!log) {
1013
return [];
1114
}
1215
log = log.trim();
16+
if (!log.includes(completeTracebackLine) && ignoreUppercaseTraceback) {
17+
return [{ text: log, highlight: false }];
18+
}
1319
if (
1420
log.startsWith('TASK ERROR') ||
1521
log.startsWith('JOB ERROR') ||
@@ -24,8 +30,6 @@ export function extractJobErrorParts(log) {
2430
return [{ text: log, highlight: false }];
2531
}
2632

27-
const completeTracebackLine = 'Traceback (most recent call last):';
28-
2933
/**
3034
* @param {string} error
3135
*/

‎src/lib/components/jobs/JobLogsModal.svelte‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,34 @@
99
let modal;
1010
/** Show/hide complete stack trace */
1111
let showDetails = false;
12+
/** @type {import('$lib/types').JobStatus} */
13+
let jobStatus;
1214
1315
/**
14-
* @param logs {string|null}
16+
* @param {import('$lib/types').ApplyWorkflow} job
1517
*/
16-
export async function show(logs) {
18+
export async function show(job) {
1719
// remove previous error
1820
if (errorAlert) {
1921
errorAlert.hide();
2022
}
21-
logParts = extractJobErrorParts(logs);
23+
jobStatus = job.status;
24+
if (job.status === 'failed') {
25+
logParts = extractJobErrorParts(job.log, true);
26+
} else {
27+
logParts = [{ text: job.log || '', highlight: false }];
28+
}
2229
modal.show();
2330
}
31+
32+
function expandDetails() {
33+
showDetails = true;
34+
// Restore focus on modal, otherwise it will not be possible to close it using the esc key
35+
const modal = document.querySelector('.modal.show');
36+
if (modal instanceof HTMLElement) {
37+
modal.focus();
38+
}
39+
}
2440
</script>
2541

2642
<Modal
@@ -33,31 +49,21 @@
3349
<svelte:fragment slot="header">
3450
<div class="flex-fill">
3551
<h1 class="h5 modal-title float-start mt-1">Workflow Job logs</h1>
36-
<button
37-
class="btn btn-secondary float-end me-3"
38-
on:click={() => (showDetails = !showDetails)}
39-
>
40-
{#if showDetails}
41-
Hide details
42-
{:else}
43-
Show details
44-
{/if}
45-
</button>
4652
</div>
4753
</svelte:fragment>
4854
<svelte:fragment slot="body">
4955
<div id="workflowJobLogsError" />
5056
<div class="row" id="workflow-job-logs">
5157
<!-- IMPORTANT: do not reindent the pre block, as it will affect the aspect of the log message -->
52-
{#if showDetails}
58+
{#if logParts.length > 1}
5359
<pre class="ps-0 pe-0">
54-
<!-- -->{#each logParts as part}<div class:highlight={part.highlight} class="ps-3 pe-3">{part.text}
55-
<!-- --></div>{/each}</pre>
60+
<!-- -->{#each logParts as part}{#if part.highlight}<div class="ps-3 pe-3 highlight">{part.text}
61+
<!-- --></div>{:else if showDetails}<div class="ps-3 pe-3">{part.text}</div>{:else}<button
62+
class="btn btn-link text-decoration-none details-btn"
63+
on:click={expandDetails}>... (details hidden, click here to expand)</button
64+
>{/if}{/each}</pre>
5665
{:else}
57-
<pre class="fw-bold">{logParts
58-
.filter((p) => p.highlight)
59-
.map((p) => p.text)
60-
.join('\n')}</pre>
66+
<pre class:highlight={jobStatus === 'failed'}>{logParts.map((p) => p.text).join('\n')}</pre>
6167
{/if}
6268
</div>
6369
</svelte:fragment>
@@ -72,4 +78,8 @@
7278
font-weight: bold;
7379
background-color: #ffe5e5;
7480
}
81+
82+
.details-btn {
83+
font-family: revert;
84+
}
7585
</style>

‎src/lib/components/jobs/JobStatusIcon.svelte‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
{#if status}
77
<span title={status}>
88
{#if status === 'submitted'}
9-
<div class="spinner-border spinner-border-sm text-primary job-status-icon-submitted" role="status">
9+
<div class="spinner-border spinner-border-sm text-primary job-status-submitted" role="status">
1010
<span class="visually-hidden">Loading...</span>
1111
</div>
1212
{:else if status === 'done'}

‎src/lib/components/jobs/JobsList.svelte‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
1414
/** @type {() => Promise<import('$lib/types').ApplyWorkflow[]>} */
1515
export let jobUpdater;
16-
/** @type {('project'|'workflow'|'user_email')[]} */
16+
/** @type {('project'|'workflow'|'user_email'|'id')[]} */
1717
export let columnsToHide = [];
1818
/** @type {boolean} */
1919
export let admin = false;
@@ -177,6 +177,9 @@
177177
<div id="jobUpdatesError" />
178178
<table class="table jobs-table">
179179
<colgroup>
180+
{#if !columnsToHide.includes('id')}
181+
<col width="40" />
182+
{/if}
180183
<col width="100" />
181184
<col width="110" />
182185
<col width="100" />
@@ -195,6 +198,9 @@
195198
</colgroup>
196199
<thead class="table-light">
197200
<tr>
201+
{#if !columnsToHide.includes('id')}
202+
<Th handler={tableHandler} key="id" label="Id" />
203+
{/if}
198204
<Th handler={tableHandler} key="status" label="Status" />
199205
<th>Options</th>
200206
<Th handler={tableHandler} key="start_timestamp" label="Start" />
@@ -213,6 +219,9 @@
213219
</tr>
214220
{#if !admin}
215221
<tr>
222+
{#if !columnsToHide.includes('id')}
223+
<th />
224+
{/if}
216225
<th>
217226
<select class="form-control" bind:value={statusFilter}>
218227
<option value="">All</option>
@@ -277,6 +286,9 @@
277286
{#if rows}
278287
{#each $rows as row}
279288
<tr class="align-middle">
289+
{#if !columnsToHide.includes('id')}
290+
<td> {row.id} </td>
291+
{/if}
280292
<td>
281293
<span>
282294
<StatusBadge status={row.status} />
@@ -293,7 +305,7 @@
293305
{#if row.status === 'failed' || row.status === 'done'}
294306
<button
295307
class="btn btn-light"
296-
on:click|preventDefault={() => jobLogsModal.show(row.log)}
308+
on:click|preventDefault={() => jobLogsModal.show(row)}
297309
>
298310
<i class="bi-list-columns-reverse" />
299311
Logs

0 commit comments

Comments
 (0)