Skip to content

Commit 4cf2c5e

Browse files
authored
Merge pull request #796 from fractal-analytics-platform/task_run_schema_update
Handled args schema changes in workflow task runs
2 parents f0583a9 + cfe0d80 commit 4cf2c5e

File tree

17 files changed

+453
-88
lines changed

17 files changed

+453
-88
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# Unreleased
44

55
* Used new API endpoint to detect task version upgrade (\#792);
6+
* Handled arguments schema changes in workflow task runs (\#796);
67

78
# 1.18.0
89

__tests__/errors.test.js

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect } from 'vitest';
2-
import { getFieldValidationError } from '../src/lib/common/errors';
2+
import { getFieldValidationError, getValidationMessagesMap } from '../src/lib/common/errors';
33

44
describe('Error utility functions', () => {
55
it('get field validation error without specifying loc', () => {
@@ -17,4 +17,41 @@ describe('Error utility functions', () => {
1717
);
1818
expect(error).eq('value is not a valid list');
1919
});
20+
21+
it('get validation errors array', () => {
22+
const errorMap = getValidationMessagesMap(
23+
{
24+
detail: [
25+
{
26+
type: 'string_too_short',
27+
loc: ['body', 'viewer_paths', 1],
28+
msg: 'String should have at least 1 character',
29+
input: '',
30+
ctx: {
31+
min_length: 1
32+
}
33+
},
34+
{
35+
type: 'value_error',
36+
loc: ['body', 'viewer_paths', 3],
37+
msg: "Value error, String must be an absolute path (given 'foobar').",
38+
input: 'foobar',
39+
ctx: {
40+
error: {}
41+
}
42+
}
43+
]
44+
},
45+
422
46+
);
47+
48+
/** @type {string[]} */
49+
const errors = /** @type {string[]} */ ((errorMap && errorMap['viewer_paths']) || []);
50+
51+
expect(errors.length).toEqual(4);
52+
expect(errors[0]).toBeUndefined();
53+
expect(errors[1]).toEqual('String should have at least 1 character');
54+
expect(errors[2]).toBeUndefined();
55+
expect(errors[3]).toEqual("Value error, String must be an absolute path (given 'foobar').");
56+
});
2057
});

__tests__/v2/AddSingleTask.test.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ describe('AddSingleTask', () => {
4747
expect.objectContaining({
4848
body: JSON.stringify({
4949
name: 'test-task',
50+
type: 'non_parallel',
5051
command_non_parallel: 'command',
5152
input_types: {},
5253
output_types: {}
@@ -81,6 +82,7 @@ describe('AddSingleTask', () => {
8182
expect.objectContaining({
8283
body: JSON.stringify({
8384
name: 'test-task',
85+
type: 'non_parallel',
8486
command_non_parallel: 'command',
8587
input_types: {},
8688
output_types: {}

components/src/lib/types/api.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,12 @@ export type HistoryUnit = {
311311

312312
export type HistoryRunAggregated = {
313313
id: number;
314+
timestamp_started: string;
315+
version: string | null;
314316
workflowtask_dump: WorkflowTaskV2;
315317
num_submitted_units: number;
316318
num_done_units: number;
317319
num_failed_units: number;
320+
args_schema_parallel: JSONSchemaObjectProperty | null;
321+
args_schema_non_parallel: JSONSchemaObjectProperty | null;
318322
};

src/lib/common/errors.js

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ function extractFieldValidationError(simpleValidationMessage, loc) {
154154
/**
155155
* @param {any} reason
156156
* @param {number | null} statusCode
157-
* @returns {null | { [key: string]: string }}
157+
* @returns {null | { [key: string]: string | string[] }}
158158
*/
159159
export function getValidationMessagesMap(reason, statusCode) {
160160
if (!isValidationError(reason, statusCode)) {
@@ -163,16 +163,25 @@ export function getValidationMessagesMap(reason, statusCode) {
163163
if (!Array.isArray(reason.detail) || reason.detail.length === 0) {
164164
return null;
165165
}
166-
/** @type {{[key: string]: string}} */
166+
/** @type {{[key: string]: string | string[]}} */
167167
const map = {};
168168
for (const error of reason.detail) {
169169
if (!hasValidationErrorPayload(error)) {
170170
return null;
171171
}
172-
if (error.loc.length !== 2 || error.loc[0] !== 'body') {
172+
if (error.loc[0] !== 'body') {
173+
return null;
174+
}
175+
if (error.loc.length === 2) {
176+
map[error.loc[1]] = error.msg;
177+
} else if (error.loc.length === 3 && typeof error.loc[2] === 'number') {
178+
if (!(error.loc[1] in map)) {
179+
map[error.loc[1]] = [];
180+
}
181+
/** @type {string[]} */ (map[error.loc[1]])[error.loc[2]] = error.msg;
182+
} else {
173183
return null;
174184
}
175-
map[error.loc[1]] = error.msg;
176185
}
177186
return map;
178187
}
@@ -294,7 +303,7 @@ export class FormErrorHandler {
294303
* Returns true if all the keys of the error map are handled by the current page or component.
295304
* Used to decide if it possible to show user friendly validation messages
296305
* or if it is necessary to display a generic error message.
297-
* @param {{[key:string]: string}} errorsMap
306+
* @param {{[key:string]: string | string[] }} errorsMap
298307
* @return {boolean}
299308
*/
300309
validateErrorMapKeys(errorsMap) {

src/lib/components/v2/projects/datasets/DatasetImagesTable.svelte

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
extraButtons
4949
} = $props();
5050
51-
let imagesStatusFilter = '';
51+
let imagesStatusFilter = $state('');
5252
5353
let showTable = $state(false);
5454
let firstLoad = true;
@@ -711,8 +711,8 @@
711711
<div class="row">
712712
<div class="col">
713713
<div class="attribute-select-wrapper mb-1">
714-
<select id="attribute-{getIdFromValue(attributeKey)}" class="invisible"
715-
></select>
714+
<select id="attribute-{getIdFromValue(attributeKey)}" class="invisible">
715+
</select>
716716
</div>
717717
</div>
718718
</div>
@@ -734,8 +734,8 @@
734734
class:btn-secondary={!applyBtnActive}
735735
>
736736
{#if searching}
737-
<span class="spinner-border spinner-border-sm" role="status" aria-hidden="true"
738-
></span>
737+
<span class="spinner-border spinner-border-sm" role="status" aria-hidden="true">
738+
</span>
739739
{/if}
740740
Apply
741741
</button>
@@ -747,8 +747,8 @@
747747
class:btn-secondary={!resetBtnActive}
748748
>
749749
{#if resetting}
750-
<span class="spinner-border spinner-border-sm" role="status" aria-hidden="true"
751-
></span>
750+
<span class="spinner-border spinner-border-sm" role="status" aria-hidden="true">
751+
</span>
752752
{/if}
753753
Reset
754754
</button>

src/lib/components/v2/tasks/AddSingleTask.svelte

Lines changed: 29 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,13 @@
44
import StandardDismissableAlert from '../../common/StandardDismissableAlert.svelte';
55
import TaskGroupSelector from './TaskGroupSelector.svelte';
66
import TypesEditor from './TypesEditor.svelte';
7-
import { detectSchemaVersion, SchemaValidator } from 'fractal-components';
7+
import {
8+
detectSchemaVersion,
9+
isCompoundType,
10+
isNonParallelType,
11+
isParallelType,
12+
SchemaValidator
13+
} from 'fractal-components';
814
915
/**
1016
* @typedef {Object} Props
@@ -36,6 +42,7 @@
3642
3743
const formErrorHandler = new FormErrorHandler('errorAlert-createTask', [
3844
'name',
45+
'type',
3946
'command_non_parallel',
4047
'command_parallel',
4148
'version',
@@ -94,6 +101,7 @@
94101
95102
const bodyData = {
96103
name,
104+
type: taskType,
97105
command_non_parallel,
98106
command_parallel,
99107
version,
@@ -350,40 +358,22 @@
350358
}}
351359
>
352360
<div class="row mb-1">
353-
<div class="col-xl-1 col-lg-2 col-3">Task type</div>
354-
<div class="col-xl-11 col-lg-8 col-9">
355-
<div class="form-check form-check-inline">
356-
<input
357-
class="form-check-input"
358-
type="radio"
359-
name="taskType"
360-
id="non_parallel"
361-
value="non_parallel"
362-
bind:group={taskType}
363-
/>
364-
<label class="form-check-label" for="non_parallel"> Non parallel </label>
365-
</div>
366-
<div class="form-check form-check-inline">
367-
<input
368-
class="form-check-input"
369-
type="radio"
370-
name="taskType"
371-
id="parallel"
372-
value="parallel"
373-
bind:group={taskType}
374-
/>
375-
<label class="form-check-label" for="parallel">Parallel</label>
376-
</div>
377-
<div class="form-check form-check-inline">
378-
<input
379-
class="form-check-input"
380-
type="radio"
381-
name="taskType"
382-
id="compound"
383-
value="compound"
384-
bind:group={taskType}
385-
/>
386-
<label class="form-check-label" for="compound">Compound</label>
361+
<div class="col-md-6 mb-2">
362+
<div class="input-group has-validation">
363+
<label for="task_type" class="input-group-text">Task type</label>
364+
<select
365+
class="form-select"
366+
bind:value={taskType}
367+
class:is-invalid={$validationErrors['type']}
368+
id="task_type"
369+
>
370+
<option value="non_parallel">Non parallel</option>
371+
<option value="parallel">Parallel</option>
372+
<option value="compound">Compound</option>
373+
<option value="converter_non_parallel">Converter Non Parallel</option>
374+
<option value="converter_compound">Converter Compound</option>
375+
</select>
376+
<span class="invalid-feedback">{$validationErrors['type']}</span>
387377
</div>
388378
</div>
389379
</div>
@@ -404,7 +394,7 @@
404394
</div>
405395
</div>
406396
</div>
407-
{#if taskType === 'non_parallel' || taskType === 'compound'}
397+
{#if isNonParallelType(taskType) || isCompoundType(taskType)}
408398
<div class="row">
409399
<div class="col-12 mb-2">
410400
<div class="input-group has-validation">
@@ -423,7 +413,7 @@
423413
</div>
424414
</div>
425415
{/if}
426-
{#if taskType === 'parallel' || taskType === 'compound'}
416+
{#if isParallelType(taskType) || isCompoundType(taskType)}
427417
<div class="row">
428418
<div class="col-12 mb-2">
429419
<div class="input-group has-validation">
@@ -468,7 +458,7 @@
468458
</div>
469459
</div>
470460
</div>
471-
{#if taskType === 'non_parallel' || taskType === 'compound'}
461+
{#if isNonParallelType(taskType) || isCompoundType(taskType)}
472462
<div class="row">
473463
<div class="col-lg-6 mb-2">
474464
<div class="input-group has-validation">
@@ -526,7 +516,7 @@
526516
</div>
527517
</div>
528518
{/if}
529-
{#if taskType === 'parallel' || taskType === 'compound'}
519+
{#if isParallelType(taskType) || isCompoundType(taskType)}
530520
<div class="row">
531521
<div class="col-lg-6 mb-2">
532522
<div class="input-group has-validation">

src/lib/components/v2/workflow/AddWorkflowTaskModal.svelte

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
import { removeIdenticalTaskGroups } from 'fractal-components/tasks/task_group_utilities';
77
import { formatMarkdown } from '$lib/common/component_utilities';
88
import { tick } from 'svelte';
9+
import { getJsonSchemaData } from 'fractal-components/jschema/jschema_initial_data';
10+
import { stripNullAndEmptyObjectsAndArrays } from 'fractal-components';
911
1012
/**
1113
* @typedef {Object} Props
@@ -57,17 +59,33 @@
5759
async () => {
5860
addingTask = true;
5961
62+
const task = await getTask(taskId);
63+
6064
const headers = new Headers();
6165
headers.set('Content-Type', 'application/json');
6266
6367
// Creating workflow task
68+
const defaultData = {};
69+
70+
if (task.args_schema_parallel) {
71+
defaultData.args_parallel = stripNullAndEmptyObjectsAndArrays(
72+
getJsonSchemaData(task.args_schema_parallel, 'pydantic_v2')
73+
);
74+
}
75+
76+
if (task.args_schema_non_parallel) {
77+
defaultData.args_non_parallel = stripNullAndEmptyObjectsAndArrays(
78+
getJsonSchemaData(task.args_schema_non_parallel, 'pydantic_v2')
79+
);
80+
}
81+
6482
const workflowTaskResponse = await fetch(
65-
`/api/v2/project/${workflow.project_id}/workflow/${workflow.id}/wftask?task_id=${taskId}`,
83+
`/api/v2/project/${workflow.project_id}/workflow/${workflow.id}/wftask?task_id=${task.id}`,
6684
{
6785
method: 'POST',
6886
credentials: 'include',
6987
headers,
70-
body: JSON.stringify({})
88+
body: JSON.stringify(defaultData)
7189
}
7290
);
7391
@@ -99,6 +117,23 @@
99117
);
100118
}
101119
120+
/**
121+
* @param {number} taskId
122+
* @returns {Promise<import('fractal-components/types/api').TaskV2>}
123+
*/
124+
async function getTask(taskId) {
125+
const response = await fetch(`/api/v2/task/${taskId}`, {
126+
method: 'GET',
127+
credentials: 'include'
128+
});
129+
130+
if (!response.ok) {
131+
throw await getAlertErrorFromResponse(response);
132+
}
133+
134+
return await response.json();
135+
}
136+
102137
/**
103138
* @param {import('fractal-components/types/api').TasksTableRow} taskRow
104139
*/

src/lib/components/v2/workflow/ArgumentsSchema.svelte

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
* @property {import('fractal-components/types/api').WorkflowTaskV2} workflowTask
3636
* @property {(wft: import('fractal-components/types/api').WorkflowTaskV2) => void} onWorkflowTaskUpdated
3737
* @property {boolean} [editable]
38+
* @property {import('fractal-components/types/jschema').JSONSchemaObjectProperty|null} argsSchemaNonParallel
39+
* @property {import('fractal-components/types/jschema').JSONSchemaObjectProperty|null} argsSchemaParallel
3840
* @property {object|undefined} [argsNonParallel]
3941
* @property {object|undefined} [argsParallel]
4042
*/
@@ -44,6 +46,8 @@
4446
workflowTask = $bindable(),
4547
onWorkflowTaskUpdated,
4648
editable = true,
49+
argsSchemaNonParallel = null,
50+
argsSchemaParallel = null,
4751
argsNonParallel = undefined,
4852
argsParallel = undefined
4953
} = $props();
@@ -206,8 +210,6 @@
206210
unsavedChangesFormBuilderNonParallel
207211
);
208212
let isSchemaValid = $derived(argsSchemaVersionValid(workflowTask.task.args_schema_version));
209-
let argsSchemaNonParallel = $derived(workflowTask.task.args_schema_non_parallel);
210-
let argsSchemaParallel = $derived(workflowTask.task.args_schema_parallel);
211213
let schemaVersion = $derived(workflowTask.task.args_schema_version);
212214
let propertiesToIgnore = $derived(getPropertiesToIgnore(false));
213215
</script>

0 commit comments

Comments
 (0)