-
Notifications
You must be signed in to change notification settings - Fork 2
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
New con-duct ls command #224
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #224 +/- ##
==========================================
- Coverage 95.36% 94.05% -1.32%
==========================================
Files 4 5 +1
Lines 626 740 +114
Branches 70 93 +23
==========================================
+ Hits 597 696 +99
- Misses 15 26 +11
- Partials 14 18 +4 ☔ View full report in Codecov by Sentry. |
Overall:
Info that seems most important:
|
note that command might be too long to show in full "by default", max memory resource i think would also be nice to know. |
@yarikoptic i've hacked together something with pyout I'm not sure how I want to handle the "include_only" list, since ideally that would be agnostic to the output format, and I don't see a clean way to incorporate with LS_SUMMARY_FORMAT. But pyout is nice and easy! |
sorry -- could you describe what you mean by |
@yarikoptic https://github.com/con/duct/pull/224/files#diff-150598b33b1982d47d2bc3f78f8ba2333963da1d5ebe2b6211cba03b7c259cfcR64 Restricting the columns to add to the table |
7fb180b
to
39e0e0f
Compare
src/con_duct/suite/ls.py
Outdated
VALUE_TRANSFORMATION_MAP = { | ||
"exit_code": "{value!E}", | ||
"wall_clock_time": "{value:.3f} sec", | ||
"peak_rss": "{value!S}", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yarikoptic if we use the custom formatter like this, the user can't provide their own format string, so its a bit less flexible than we did for regular duct output, but I think its a whole lot cleaner/dryer. And the user experience seems better too-- ie, provide an ordered list of fields that you want, rather than a format string that requires knowledge of how the formatter works.
If you like this approach too, I'll fill out this map with all the fields, and perhaps refactor regular duct output to function this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get some way done, let us try and then lets discuss and potentially improve.
Eventually I would expect us to be able to extend list of relevant attributes to be visualized in ls
based on user's inputs. Might even be some kind of a "run a regex across stdout or stderr to get information of interest to be printed in each row" to support some custom extraction but for that I do not have immediate use case hence just a wild idea (must not be yet implemented anyhow).
Some significantly improved UX/rendering IMO! Heres some samples of what it looks like now
|
we should see what we should display in "LOG_PREFIX",
so we display PREFIX as loaded from the files. But of interest here likely is actually "FILES_PREFIX" based on the filenames loaded. |
98bad79
to
896972a
Compare
Fixes: con#185
🚀 PR was released in |
Please do not forget next time to make PR titles descriptive enough to serve as a changelog entry. |
Fixes #185
summaries
yaml
json
jsonpp
TODO