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

make flux resource drain -o long reason expandable #6338

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

garlick
Copy link
Member

@garlick garlick commented Oct 2, 2024

Problem: the "long" format of 'flux resource drain' doesn't show the un-truncated drain reason, but one might expect that to be the case.

Now that we have expandable field width support, use it for {reason} in the "long" format. In addition, to conserve space, {timestamp} is changed to use same format as "default", since the longer representation wasn't particularly helpful.

$ flux resource drain
TIME         STATE    REASON                         NODELIST
Oct02 13:06  drained  This is a long drain message + pop-os
$ flux resource drain -o long
TIME         STATE    RANKS    REASON                                NODELIST
Oct02 13:06  drained  0        This is a long drain message isnt it? pop-os

Same for {reason} in the "long" format of 'flux resource status':

$ flux resource status -o long
       STATE UP NNODES REASON                                NODELIST
     drained  ✔      1 This is a long drain message isnt it? pop-os

Problem: the "long" format of 'flux resource drain' doesn't show the
un-truncated drain reason, but one might expect that to be the case.

Now that we have expandable field width support, use it for {reason}
in the "long" format.  In addition, to conserve space, {timestamp} is
changed to use same format as "default", since the longer representation
wasn't particularly helpful.

$ flux resource drain
TIME         STATE    REASON                         NODELIST
Oct02 13:06  drained  This is a long drain message + pop-os
$ flux resource drain -o long
TIME         STATE    RANKS    REASON                                NODELIST
Oct02 13:06  drained  0        This is a long drain message isnt it? pop-os

Same for {reason} in the "long" format of 'flux resource status':

$ flux resource status -o long
       STATE UP NNODES REASON                                NODELIST
     drained  ✔      1 This is a long drain message isnt it? pop-os
@wihobbs
Copy link
Member

wihobbs commented Oct 2, 2024

Ahh thanks I was just looking for this the other day!

Copy link
Contributor

@grondo grondo left a comment

Choose a reason for hiding this comment

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

Nice improvement! 👍

@garlick
Copy link
Member Author

garlick commented Oct 2, 2024

Thanks! Setting MWP.

@mergify mergify bot merged commit 828de38 into flux-framework:master Oct 2, 2024
33 checks passed
Copy link

codecov bot commented Oct 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.33%. Comparing base (367243a) to head (e5f695e).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6338      +/-   ##
==========================================
- Coverage   83.34%   83.33%   -0.02%     
==========================================
  Files         523      523              
  Lines       86183    86183              
==========================================
- Hits        71833    71819      -14     
- Misses      14350    14364      +14     
Files with missing lines Coverage Δ
src/cmd/flux-resource.py 94.97% <ø> (ø)

... and 8 files with indirect coverage changes

@garlick garlick deleted the long_drain branch October 2, 2024 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants