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.

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

Add NodeState enum #2966

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

mr0re1
Copy link
Collaborator

@mr0re1 mr0re1 commented Aug 22, 2024

Add proper enum to reduce usage of error-prone string literals (follow up action).

@mr0re1 mr0re1 added the release-chore To not include into release notes label Aug 22, 2024
@mr0re1 mr0re1 requested a review from abbas1902 August 22, 2024 00:47
@mr0re1 mr0re1 enabled auto-merge August 22, 2024 00:48
assert NodeState.DOWN != NodeState.POWER_DOWN
assert NodeState.DOWN != NodeState("dOwN") # case sensitive

assert f"{NodeState.DOWN}" == "NodeState.DOWN"
Copy link
Contributor

Choose a reason for hiding this comment

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

This assertion seems to be failing in pre-commit, yet works fine while testing on my local env (based on the error message seems to be comparing this as 'DOWN' == 'NodeState.Down'). Maybe try running it again?

def __repr__(self):
return self.__str__()

# *** Base states
Copy link
Contributor

Choose a reason for hiding this comment

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

Will other node states like unknown or end be added or are they not needed? (from https://github.com/SchedMD/slurm/blob/master/slurm/slurm.h#L980)

@abbas1902 abbas1902 assigned mr0re1 and unassigned abbas1902 Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-chore To not include into release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants