Skip to content

Conversation

@jkellerer
Copy link
Collaborator

This PR is a super-light version of outstanding PR #207 to control when command output is sent to logs, console or both, with a default of "auto" so that interactive usage isn't fully silent when log is specified (e.g. in global settings).

root@log:/# ./resticprofile --log=test.log containers.check
using temporary cache in /tmp/restic-check-cache-3409750633
create exclusive lock for repository
load indexes
check all packs
check snapshots, trees and blobs
[3:13] 100.00%  580 / 580 snapshots

no errors were found
root@log:/# cat test.log 
2024/03/16 22:12:12 INFO  using configuration file: /etc/resticprofile/profiles.yaml
2024/03/16 22:12:12 INFO  profile 'containers': initializing repository (if not existing)
2024/03/16 22:12:12 INFO  profile 'containers': starting 'check'
using temporary cache in /tmp/restic-check-cache-3409750633
create exclusive lock for repository
load indexes
check all packs
check snapshots, trees and blobs
[3:13] 100.00%  580 / 580 snapshots

no errors were found

.. PR #207 is still required to have proper terminal output in this case but it is better than none.

@jkellerer
Copy link
Collaborator Author

@creativeprojects, let me know what you think about this and whether names need to be adjusted.

@creativeprojects
Copy link
Owner

Yes, this is a very good idea.

I'm not too sure about the name of the flag either. We could also do:

  • log-output (with something like dual, log-only) ?
  • log-console (boolean) ?

🤔

@jkellerer
Copy link
Collaborator Author

jkellerer commented Mar 17, 2024

name was also one thing were I wasn't sure yet. Had used log-commands from default-command.

Maybe add-output-to (e.g. --add-output-to=log, --add-output-to=console, --add-output-to=all)

Mostly the flag wont be needed as the auto value should be fine. Also we have some commands where the output must not be sent to logs. It may be good to have a name that doesn't get in the way if output cannot be added.

@jkellerer jkellerer added this to the v0.27.0 milestone Mar 17, 2024
@jkellerer jkellerer force-pushed the ft/log-commands-switch branch from 0b14ca0 to 68c0331 Compare March 19, 2024 18:33
@jkellerer jkellerer force-pushed the ft/log-commands-switch branch from 68c0331 to 519d4b3 Compare March 19, 2024 18:38
@jkellerer jkellerer marked this pull request as ready for review March 19, 2024 18:51
@jkellerer
Copy link
Collaborator Author

Changed the name to "command-output", kind of a combination of yours and mine proposals :)

resticprofile --log=rp.log --command-output=log ...
resticprofile --log=rp.log --command-output=console ...
resticprofile --log=rp.log --command-output=all ...
resticprofile --log=rp.log --command-output=auto ...

# also valid
resticprofile --log=rp.log --command-output=log,console ... 

@codecov
Copy link

codecov bot commented Mar 21, 2024

Codecov Report

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

Project coverage is 71.07%. Comparing base (667180e) to head (4061506).
Report is 1 commits behind head on master.

Files Patch % Lines
commands.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #343      +/-   ##
==========================================
+ Coverage   71.03%   71.07%   +0.05%     
==========================================
  Files         121      121              
  Lines       12378    12394      +16     
==========================================
+ Hits         8792     8809      +17     
+ Misses       3190     3189       -1     
  Partials      396      396              
Flag Coverage Δ
unittests 71.07% <92.86%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jkellerer jkellerer force-pushed the ft/log-commands-switch branch from 4061506 to 5bf33d7 Compare March 21, 2024 18:00
@jkellerer jkellerer merged commit 85d5afc into creativeprojects:master Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants