support expandable width output formats and use them in flux resource list to avoid truncation of queue field
#6284
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
garlick
approved these changes
Sep 15, 2024
src/bindings/python/flux/util.py
Outdated
Comment on lines
842
to
843
| # Save a format without any spec to allow deterimination of | ||
| # maximum width after formatting all items: |
There was a problem hiding this comment.
s/deterimination/determination/
There was a problem hiding this comment.
Thanks! Fixed that and I also dropped the flux-pgrep codeql commit since it didn't help workaround the codeql false positive. Will set MWP now.
grondo
force-pushed
the
expanding-formats
branch
from
578849f
to
f33efbf
Compare
Problem: When generating formatted output, there are times where truncating a wide field is not desirable, for example when a comma separated list is being displayed, truncation can completely hide some entries. However, the only current alternative is to use a very wide field in the output format by default, which is also not desirable since it wastes horizontal space to handle an uncommon case. Extend the current sentinel `?:`, which specifies that a field is to be collapsed if the output width for all entries is 0, with a new sentinel `+:`, which indicates the field width should be increased to handle the largest width of all displayed items. To take both actions `?+:` is also supported. Extend `OutputFormat.filter_empty()` to handle these new sentinels and modify the format spec if necessary before returning the new format to the caller.
Problem: The name of the filter_empty() method of the OutputFormat class is misleading since it now does more than just filter empty fields. Rename the method to just filter(). Update callers.
Problem: There are no tests that ensure the width adjustment sentinels `+:` and `?+:` are functional. Add some tests in t2800-jobs-cmd.t to ensure these sentinels work as expected.
Problem: The expandable format sentinels `+:` and `?+:` are not documented anywhere. Add a description to the `OUTPUT FORMAT` section in flux-jobs(1).
Problem: The fixed width of the queue field in `flux resource list` causes confusion for users with overlapping queues because the queues are joined into a comma separated list and truncation could cause some queues to be completely left out of the field. Now that the OutputFormat class supports "expandable" fields, use this with the `queue` field in `flux resource list` so that no truncation is guaranteed without using a large width by default. Also apply the expandable field to ngpus to save some horizontal space in the common case.
grondo
force-pushed
the
expanding-formats
branch
from
f33efbf
to
a0cea0f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6284 +/- ##
=======================================
Coverage 83.36% 83.36%
=======================================
Files 522 522
Lines 85985 86036 +51
=======================================
+ Hits 71682 71726 +44
- Misses 14303 14310 +7
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds two new output format "sentinels"
+:and?+:to theOutputFormatclass that allow automated adjustment of the field width to avoid truncation.+:only does the expansion, while?+:combines the functionality of+:and?:and expands the field to the maximum observed width and removes the field if all entries would result in an "empty" value.Then
?+:is applied toflux resource listdefault output forqueueandngpusfields. This avoids truncation of the QUEUE column, which may contain comma separated list of queues on system instances (and in the case ofngpussaves a couple characters of width in the common case)