-
Notifications
You must be signed in to change notification settings - Fork 124
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 a helper for saving command output in a file #3286
base: main
Are you sure you want to change the base?
Conversation
99f2692
to
4b6e215
Compare
b3f7c66
to
8eff501
Compare
8eff501
to
4dcd596
Compare
Some problem on TF side?
Let's reschedule? |
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.
Nice!
4dcd596
to
1b50f67
Compare
tmt/utils/__init__.py
Outdated
yield from output_lines | ||
yield OUTPUT_WIDTH * '~' | ||
yield f'{comment_sign} {name} ({line_summary} lines)' | ||
yield comment_sign + (OUTPUT_WIDTH - 1) * '~' |
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.
yield comment_sign + (OUTPUT_WIDTH - 1) * '~' | |
yield f'{comment_sign} ' + (OUTPUT_WIDTH - 2) * '~' |
Would you be ok with this as well? I find it a little bit more pleasing to the eye...
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.
I don't mind, just need to fix tests after such a change.
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.
That would be nice, thanks!
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.
Addressed in 21f1896
This will be a very needed functionality: plugin runs a command, and its output needs to be saved in a dedicated file, usualy because the output represents some interesting, standalone piece of information. For example, an AVC denial report generated by `ausearch`. If it would be the only such actor, it would be fine to keep the implementation in the `avc` plugin. But, with the advent of saving results of `prepare` and `finish` phases, the question is, if I get `results.yaml` for a `prepare` step, would it even contain anything besides `pass` or `error`? Well, it can contain logs of commands like `ansible-playbook` or shell scripts. And suddenly we have several plugins that may need to save some walls of texts, usually produced by a command, in files, and then link those files to results and present them to user. And because of that, it makes sense to provide a helper function that would produce unified output for all of them? Why should `avc` separate stdout and stderr by two lines and `prepare/shell` with just one? Not on my watch.
1b50f67
to
21f1896
Compare
This will be a very needed functionality: the plugin runs a command, and its output needs to be saved in a dedicated file, usually because the output represents some interesting, standalone piece of information. For example, an AVC denial report generated by
ausearch
.If it were the only such actor, it would be fine to keep the implementation in the
avc
plugin. But, with the advent of saving results of theprepare
andfinish
phases, the question is, if I getresults.yaml
for aprepare
step, would it even contain anything besidespass
orerror
? Well, it can contain logs of commands likeansible-playbook
or shell scripts. And suddenly we have several plugins that may need to save some walls of text, usually produced by a command, in files, and then link those files to results and present them to a user.And because of that, it makes sense to provide a helper function that would produce unified output for all of them. Why should
avc
separate stdout and stderr by two lines andprepare/shell
with just one? Not on my watch.Pull Request Checklist