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

flux-hostlist: allow idset argument to --nth and --exclude options #6478

Merged
merged 5 commits into from
Dec 5, 2024

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Dec 5, 2024

This PR makes the following minor improvements to flux hostlist:

  • -n, --nth=IDS: This option now takes an idset instead of a single index into the hostlist. This allows a user to get a "slice" of a hostlist while preserving current behavior, e.g. flux hostlist -n 0-1 JOBID to get the first two hosts of a job.

  • -x, --exclude=HOSTS|IDS: As suggested by @vsoch, this option now takes either a hostlist or idset or indices to exclude from the hostlist. This allows a user to exclude the first host for example with flux hostlist -x 0 local

Fixes #6464

@grondo grondo force-pushed the flux-hostlist-tweaks branch 2 times, most recently from 46b94b8 to ffa8820 Compare December 5, 2024 14:53
src/cmd/flux-hostlist.py Fixed Show fixed Hide fixed
Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

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

LGTM!

@grondo grondo force-pushed the flux-hostlist-tweaks branch from ffa8820 to bf3a55e Compare December 5, 2024 15:51
@grondo
Copy link
Contributor Author

grondo commented Dec 5, 2024

Thanks! Fixed the issue codeql flagged and forced a push. Will set MWP.

Problem: There is no way to return a set of hosts at multiple indices
in a hostlist with the flux-hostlist command.

Enhance the `-n, --nth` option to accept an idset instead of a
single integer.
Problem: There is no way to exclude hosts from a hostlist by index
instead of hostname.

Enhance `-x, --exclude` to take an idset of indices to remove in
addition to the current support for a list of hosts.
Problem: The flux-hostlist `-n, --nth` option no longer needs to be
in the mutually exclusive group with --expand and --count.

Remove `-n, --nth` from the group so it can be used with these options.
Problem: The tests for the flux hostlist command are lacking coverage
of some options.

Update t2814-hostlist-cmd.t to cover new functionality.
Problem: The flux-hostlist(1) manual is out of date with resepect
to recent changes to the command.

Update the relevant documentation in flux-hostlist(1).
@grondo grondo force-pushed the flux-hostlist-tweaks branch from bf3a55e to 974dda0 Compare December 5, 2024 15:57
@mergify mergify bot merged commit f761aec into flux-framework:master Dec 5, 2024
34 checks passed
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.

Project coverage is 83.59%. Comparing base (861c5cf) to head (974dda0).
Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
src/cmd/flux-hostlist.py 84.61% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6478      +/-   ##
==========================================
- Coverage   83.61%   83.59%   -0.03%     
==========================================
  Files         524      524              
  Lines       87485    87493       +8     
==========================================
- Hits        73148    73137      -11     
- Misses      14337    14356      +19     
Files with missing lines Coverage Δ
src/cmd/flux-hostlist.py 97.31% <84.61%> (+0.12%) ⬆️

... and 12 files with indirect coverage changes

@grondo grondo deleted the flux-hostlist-tweaks branch December 5, 2024 16:54
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.

flux hostlist: request idset with -x or --exclude
2 participants