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.

Sign up for GitHub

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

Replace min/max CPU fields with single CPU cores field #616

Merged
merged 5 commits into from
Jan 25, 2025
Merged

Replace min/max CPU fields with single CPU cores field #616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d6f96d6
Replace min/max CPU fields with single CPU cores field
suemkwon Jan 22, 2025
d58e2c8
refactor: consolidate CPU cores fields to single max_cpu_cores
suemkwon Jan 24, 2025
77c8cad
Update FileViewer.js
suemkwon Jan 24, 2025
d95e148
pdate CPU cores validation and remove selectMins label
suemkwon Jan 24, 2025
17bdb47
Merge remote-tracking branch 'origin/main' into CPU-request-fields
suemkwon Jan 25, 2025
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
1 change: 1 addition & 0 deletions public/static/locales/en/launch.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"browse": "Browse",
"clearInput": "Clear Input",
"comments": "Comments",
"cpuCores": "CPU Cores",
"delete": "Delete",
"errorLoadingReferenceGenomes": "Error loading Reference Genomes",
"errorResourceDoesNotExist": "The selected file/folder does not exist.",
Expand Down
61 changes: 8 additions & 53 deletions src/components/apps/launch/ResourceRequirements.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,16 +119,11 @@ const StepResourceRequirementsForm = ({
return (
<div style={{ margin: 8 }}>
<Grid container spacing={1}>
<Grid item xs={12}>
<Typography variant="subtitle2">
{t("selectMins")}
</Typography>
</Grid>
<Grid item xs={12}>
<FastField
id={buildID(baseId, ids.RESOURCE_REQUESTS.TOOL_CPU)}
name={`requirements.${index}.min_cpu_cores`}
label={t("minCPUCores")}
name={`requirements.${index}.max_cpu_cores`}
label={t("cpuCores")}
Copy link
Member

Choose a reason for hiding this comment

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

I would also remove the selectMins grid item just above this, since these are no longer all minimum requirements.

component={FormSelectField}
>
{cpuCoreList.map((size, index) => (
Expand Down Expand Up @@ -172,28 +167,6 @@ const StepResourceRequirementsForm = ({
</FastField>
</Grid>
</Grid>
<br />
<Grid container spacing={1}>
<Grid item xs={12}>
<Typography variant="subtitle2">
{t("selectMaxes")}
</Typography>
</Grid>
<Grid item xs={12}>
<FastField
id={buildID(baseId, "MAX_CPU")}
name={`requirements.${index}.max_cpu_cores`}
label={t("maxCPUCores")}
component={FormSelectField}
>
{cpuCoreList.map((size, index) => (
<MenuItem key={index} value={size}>
{size}
</MenuItem>
))}
</FastField>
</Grid>
</Grid>
</div>
);
};
Expand Down Expand Up @@ -320,27 +293,16 @@ const StepResourceRequirementsReview = ({
showAll,
}) => {
const { t } = useTranslation("launch");
const {
step_number,
min_cpu_cores,
min_memory_limit,
min_disk_space,
max_cpu_cores,
} = stepRequirements;
const { step_number, min_memory_limit, min_disk_space, max_cpu_cores } =
stepRequirements;

const hasRequest = !!(
min_cpu_cores ||
min_memory_limit ||
min_disk_space ||
max_cpu_cores
);
const hasRequest = !!(min_memory_limit || min_disk_space || max_cpu_cores);

return (
(showAll || hasRequest) && (
<Accordion
defaultExpanded={
!!(
stepRequirementErrors?.min_cpu_cores ||
stepRequirementErrors?.min_memory_limit ||
stepRequirementErrors?.min_disk_space ||
stepRequirementErrors?.max_cpu_cores
Expand Down Expand Up @@ -372,11 +334,11 @@ const StepResourceRequirementsReview = ({
<Table>
<TableBody>
<ResourceRequirementsReviewRow
label={t("minCPUCores")}
value={min_cpu_cores}
label={t("cpuCores")}
value={max_cpu_cores}
valueFormatter={(value) => value}
showAll={showAll}
error={stepRequirementErrors?.min_cpu_cores}
error={stepRequirementErrors?.max_cpu_cores}
/>
<ResourceRequirementsReviewRow
label={t("minMemory")}
Expand All @@ -396,13 +358,6 @@ const StepResourceRequirementsReview = ({
stepRequirementErrors?.min_disk_space
}
/>
<ResourceRequirementsReviewRow
label={t("maxCPUCores")}
value={max_cpu_cores}
valueFormatter={(value) => value}
showAll={showAll}
error={stepRequirementErrors?.max_cpu_cores}
/>
</TableBody>
</Table>
</TableContainer>
Expand Down
37 changes: 22 additions & 15 deletions src/components/apps/launch/formatters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,21 +198,28 @@ const formatSubmission = (
requirements,
groups,
}
) => ({
notify,
notify_periodic: notifyPeriodic,
periodic_period: periodicPeriod,
debug,
create_output_subdir: output_dir === defaultOutputDir,
name: name.trim(),
description,
output_dir,
system_id,
app_id,
app_version_id,
requirements,
config: groups?.reduce(paramConfigsReducer, {}),
});
) => {
const formattedRequirements = requirements.map((req) => ({
...req,
min_cpu_cores: req.max_cpu_cores,
}));

return {
notify,
notify_periodic: notifyPeriodic,
periodic_period: periodicPeriod,
debug,
create_output_subdir: output_dir === defaultOutputDir,
name: name.trim(),
description,
output_dir,
system_id,
app_id,
app_version_id,
requirements: formattedRequirements,
config: groups?.reduce(paramConfigsReducer, {}),
};
};

/**
* Appends the given group's parameter values to the given submission config.
Expand Down
2 changes: 1 addition & 1 deletion src/components/apps/launch/validate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ const validate = (t, hasParams) => (values) => {
t
);
if (err) {
reqErrors[i] = { min_cpu_cores: err };
reqErrors[i] = { max_cpu_cores: err };
}
}
});
Expand Down
Loading