Skip to content

Remove default Preemption configuration#212

Open
elelaysh wants to merge 1 commit into
masterfrom
feat/rm-PreemptType
Open

Remove default Preemption configuration#212
elelaysh wants to merge 1 commit into
masterfrom
feat/rm-PreemptType

Conversation

@elelaysh
Copy link
Copy Markdown
Contributor

@elelaysh elelaysh commented Mar 2, 2026

We should let sites decide what preemption they want and set additional settings like PreemptMode, PreemptExemptTime.

We should let sites decide what preemption they want
and set additional settings like PreemptMode, PreemptExemptTime.
@elelaysh elelaysh requested a review from a team as a code owner March 2, 2026 13:16
@elelaysh elelaysh changed the title Remove PreemptType=preempt/partition_prio Remove default Preemption configuration Mar 2, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request removes the default Slurm preemption configuration (PreemptType and PreemptMode) from the Ansible role's defaults, with the goal of making the role less opinionated and allowing sites to configure preemption as needed. My review includes one suggestion to also remove a related configuration parameter for consistency and to simplify the default configuration.

I am having trouble creating individual review comments. Click here to see my feedback.

defaults/main.yml (29-30)

medium

To be consistent with removing opinionated preemption settings, consider also removing PriorityWeightPartition: 1000 from line 28. This setting is only effective when PriorityType=priority/multifactor is configured, which is not the default in Slurm or this role. Removing it would further simplify the default configuration and align with the goal of this PR.

@elelaysh elelaysh requested a review from sjpb March 2, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant