Skip to content

Allow Feature specification in compute resources #6962

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

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Allow Feature specification in compute resources #6962

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c745672
Allow `Feature` specification in compute resources
efim-a-efim Sep 4, 2025
eefbee3
Merge branch 'develop' into develop
efim-a-efim Sep 4, 2025
cd4ce84
Merge branch 'develop' into develop
efim-a-efim Sep 8, 2025
8bf168b
Merge branch 'develop' into develop
efim-a-efim Sep 10, 2025
80b3397
Merge branch 'develop' into develop
efim-a-efim Sep 15, 2025
c695a27
Merge branch 'develop' into develop
efim-a-efim Sep 18, 2025
b760efb
Merge branch 'develop' into develop
efim-a-efim Sep 20, 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
2 changes: 1 addition & 1 deletion cli/src/pcluster/validators/slurm_settings_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"Global": ["nodes", "partitionname", "resumetimeout", "state", "suspendtime", "oversubscribe"],
},
"ComputeResource": {
"Global": ["cpus", "features", "gres", "nodeaddr", "nodehostname", "nodename", "state", "weight"],
"Global": ["cpus", "gres", "nodeaddr", "nodehostname", "nodename", "state", "weight"],
Copy link
Contributor

Choose a reason for hiding this comment

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

[Testing] This change is missing coprresponding changes in unit tests and integration tests.
The reason for missing unit test is that CustomSlurmSettingsValidator takes as input the deny list and we unit test here that a generic deny list given as input is honored.

To improve the testing we need to extend the unit test here to verify that the validator is called with the expected deny list.

The feature captured by this PR and aws/aws-parallelcluster-cookbook#3020, which is its counterpart in the cookbook, requires the extension of the existing end to end test here

We will evaluate internally if we can consider the integration tests out of scope for this PR and be us taking care of it. Will get back to this by tomorrow eod of the day.

Also, have you manually tested this PR together with with aws/aws-parallelcluster-cookbook#3020, which is its required counterpart in the cookbook?

Copy link
Contributor

Choose a reason for hiding this comment

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

[Documentation] this change requires an entry in our changelog. Can you please add

3.15.0
------

**ENHANCEMENTS**
- Allow specification of `Features` as custom Slurm setting for compute resources.

},
}

Expand Down