Skip to content
Open
42 changes: 42 additions & 0 deletions client/src/components/Applications/AppForm/AppForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,48 @@ export const AppSchemaForm = ({ app }) => {
) {
job.memoryMB = queue.maxMemoryMB;
}
if (isAppUsingDynamicExecSystem(app)) {
if (!job.parameterSet.schedulerOptions) {
job.parameterSet.schedulerOptions = [];
}
// pick the right profile for the right exec system

const isValidDynamicExecSystems =
Array.isArray(app.definition.notes.dynamicExecSystems) &&
app.definition.notes.dynamicExecSystems.every(
(s) => typeof s === 'object' && s !== null && 'systemId' in s
);
if (!isValidDynamicExecSystems) {
return (
<div
id="appDetail-wrapper"
className="has-message appDetail-error"
>
<SectionMessage type="warning">
{'DynamicExecSystems for this application are invalid'}
</SectionMessage>
</div>
);
}

const selectedSystem = app.definition.notes.dynamicExecSystems.find(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the dynamicExecSystems field does not correspond to our expectations as an array of objects, we'll need to handle those cases here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a check for isValidDynamicExecSystems

(s) => s.systemId === job.execSystemId
);
const profileName = selectedSystem?.profileName;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no profile name is found, we'll want to skip the .push below right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, good catch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a check for profileName

if (!!profileName) {
job.parameterSet.schedulerOptions = [
...job.parameterSet.schedulerOptions.filter(
(opt) => !opt.arg.startsWith('--tapis-profile')
),
{
name: 'TACC Scheduler Profile',
arg: `--tapis-profile ${profileName}`,
description: 'Scheduler profile for HPC clusters at TACC',
include: true,
},
];
}
}

// Add allocation scheduler option
if (job.allocation) {
Expand Down
58 changes: 54 additions & 4 deletions client/src/components/Applications/AppForm/AppForm.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -341,10 +341,10 @@ describe('AppSchemaForm', () => {
expect(execSystemDropDown.value).toBe('frontera');
const options = Array.from(execSystemDropDown.querySelectorAll('option'));
const actualValues = Array.from(options).map((option) => option.value);
const expectedValuesWithEmpty = [
'',
...executionSystemNotesFixture['dynamicExecSystems'],
];
const systemIds = executionSystemNotesFixture['dynamicExecSystems'].map(
(s) => s.systemId
);
const expectedValuesWithEmpty = ['', ...systemIds];
expect(actualValues).toEqual(
expect.arrayContaining(expectedValuesWithEmpty)
);
Expand Down Expand Up @@ -414,6 +414,56 @@ describe('AppSchemaForm', () => {
expect(execSystemDropDown.value).toBe('ls6');
});

it('uses correct dynamic scheduler profile for the assigned dynamic exec system', async () => {
const store = mockStore({
...initialMockState,
});

const appFixture = {
...helloWorldAppFixture,
definition: {
...helloWorldAppFixture.definition,
notes: {
...helloWorldAppFixture.definition.notes,
...executionSystemNotesFixture,
},
},
execSystems: execSystemsFixture,
};
const { getByText } = renderAppSchemaFormComponent(store, appFixture);

const payload = {
...helloWorldAppSubmissionPayloadFixture,
job: {
...helloWorldAppSubmissionPayloadFixture.job,
name: `hello-world-0.0.1_${frozenDate}T00:00:00`,
parameterSet: {
...helloWorldAppSubmissionPayloadFixture.job.parameterSet,
schedulerOptions: [
{
arg: '--tapis-profile tacc-apptainer',
description: 'Scheduler profile for HPC clusters at TACC',
include: true,
name: 'TACC Scheduler Profile',
},
...helloWorldAppSubmissionPayloadFixture.job.parameterSet
.schedulerOptions,
],
},
},
};

const submitButton = getByText(/Submit/);
fireEvent.click(submitButton);

await waitFor(() => {
expect(store.getActions()).toEqual([
{ type: 'GET_SYSTEM_MONITOR' },
{ type: 'SUBMIT_JOB', payload: payload },
]);
});
});

it('does not display exec system when dynamic exec system is not enabled', async () => {
const store = mockStore({
...initialMockState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,6 @@ export const helloWorldAppFixture = {
],
containerArgs: [],
schedulerOptions: [
{
arg: '--tapis-profile tacc',
name: 'TACC Scheduler Profile',
description: 'Scheduler profile for HPC clusters at TACC',
inputMode: 'FIXED',
notes: {},
},
{
arg: '--job-name ${JobName}',
name: 'Slurm job name',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,5 +606,10 @@ export const execSystemsFixture = [
];

export const executionSystemNotesFixture = {
dynamicExecSystems: ['maverick2', 'frontera', 'ls6', 'vista'],
dynamicExecSystems: [
{ systemId: 'maverick2', profileName: 'tacc-apptainer' },
{ systemId: 'frontera', profileName: 'tacc-apptainer' },
{ systemId: 'ls6', profileName: 'tacc-apptainer' },
{ systemId: 'vista', profileName: 'tacc-apptainer' },
],
};
7 changes: 6 additions & 1 deletion client/src/utils/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ export type TAppFileInput = {
targetPath?: string;
};

type TAppNotesDynamicExecSystems = {
systemId: string;
profileName: string;
};

type TAppNotes = {
label?: string;
shortLabel?: string;
Expand All @@ -59,7 +64,7 @@ type TAppNotes = {
isInteractive?: boolean;
hideNodeCountAndCoresPerNode?: boolean;
icon?: string;
dynamicExecSystems?: string[];
dynamicExecSystems?: TAppNotesDynamicExecSystems[];
queueFilter?: string[];
hideQueue?: boolean;
hideAllocation?: boolean;
Expand Down
3 changes: 2 additions & 1 deletion server/portal/apps/workspace/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ def _get_app(app_id, app_version, user):

data = {'definition': app_def}

exec_systems = getattr(app_def.notes, 'dynamicExecSystems', [])
dynamic_exec_systems = getattr(app_def.notes, "dynamicExecSystems", [])
exec_systems = [system.systemId for system in dynamic_exec_systems]
if len(exec_systems) > 0:
data['execSystems'] = _get_exec_systems(user, exec_systems)
else:
Expand Down
5 changes: 4 additions & 1 deletion server/portal/apps/workspace/api/views_unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,10 @@ def test_get_app_dynamic_exec_sys(
) as f:
app = json.load(f)
if dynamic_exec_system:
app["notes"]["dynamicExecSystems"] = ["frontera", "ls6"]
app["notes"]["dynamicExecSystems"] = [
{"systemId": "frontera", "profileName": "tacc-apptainer"},
{"systemId": "ls6", "profileName": "tacc-apptainer"},
]

# setup tapis mocks
mock_tapis_client.apps.getApp.return_value = TapisResult(**app)
Expand Down
Loading