By default only brief logging output at verbosity 1 #283

Merged
witten merged 5 commits from palto42/borgmatic:list-files into master 2020-01-22 20:36:59 +00:00
8 changed files with 57 additions and 25 deletions

3
NEWS
View File

@ -1,5 +1,8 @@
1.4.23.dev0
* #274: Add ~/.config/borgmatic.d as another configuration directory default.
* Removed `borg --list --stats` option from `create`and `prune` actions at verbosity 1
* The `--stats` now always requires to use the `borgmatic --stats` option to be enabled.
* New option `--files` to (re-)add the `borg` `--list` at verbosity 1
1.4.22
* #276, #285: Disable colored output when "--json" flag is used, so as to produce valid JSON ouput.

View File

@ -131,6 +131,7 @@ def create_archive(
progress=False,
stats=False,
json=False,
files=False,
):
'''
Given vebosity/dry-run flags, a local or remote repository path, a location config dict, and a
@ -177,13 +178,13 @@ def create_archive(
+ (('--lock-wait', str(lock_wait)) if lock_wait else ())
+ (
('--list', '--filter', 'AME-')
if logger.isEnabledFor(logging.INFO) and not json and not progress
if (files or logger.isEnabledFor(logging.DEBUG)) and not json and not progress
else ()
)
+ (('--info',) if logger.getEffectiveLevel() == logging.INFO and not json else ())
Review

Potential simplification: You can probably remove the if logger.isEnabledFor(logging.INFO) check, given that Borg's --list --filter works at verbosity 0, and it's now opt-in. Example revision:

if (files or logger.isEnabledFor(logging.DEBUG))
and not json
and not progress
Potential simplification: You can probably remove the `if logger.isEnabledFor(logging.INFO)` check, given that Borg's `--list --filter` works at verbosity 0, and it's now opt-in. Example revision: ``` if (files or logger.isEnabledFor(logging.DEBUG)) and not json and not progress ```
Review

Agree, will change as suggested.

Agree, will change as suggested.
+ (
('--stats',)
if not dry_run and (logger.isEnabledFor(logging.INFO) or stats) and not json
if (stats or logger.isEnabledFor(logging.DEBUG)) and not json and not dry_run
else ()
)
+ (('--debug', '--show-rc') if logger.isEnabledFor(logging.DEBUG) and not json else ())
@ -207,8 +208,6 @@ def create_archive(
if json:
output_log_level = None
elif stats:
output_log_level = logging.WARNING
else:
output_log_level = logging.INFO

View File

@ -41,6 +41,7 @@ def prune_archives(
local_path='borg',
remote_path=None,
stats=False,
files=False,
):
'''
Given dry-run flag, a local or remote repository path, a storage config dict, and a
@ -57,17 +58,13 @@ def prune_archives(
+ (('--remote-path', remote_path) if remote_path else ())
+ (('--umask', str(umask)) if umask else ())
+ (('--lock-wait', str(lock_wait)) if lock_wait else ())
+ (('--stats',) if not dry_run and logger.isEnabledFor(logging.INFO) else ())
+ (('--info', '--list') if logger.getEffectiveLevel() == logging.INFO else ())
+ (('--debug', '--list', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ())
+ (('--stats',) if (stats or logger.isEnabledFor(logging.DEBUG)) and not dry_run else ())
+ (('--info',) if logger.getEffectiveLevel() == logging.INFO else ())
+ (('--list',) if files or logger.isEnabledFor(logging.DEBUG) else ())
Review

Would be good to parenthesize the logger.getEffectiveLevel() == logging.DEBUG or stats part for clarity.

Would be good to parenthesize the `logger.getEffectiveLevel() == logging.DEBUG or stats` part for clarity.
+ (('--debug', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ())
+ (('--dry-run',) if dry_run else ())
+ (('--stats',) if stats else ())
+ (tuple(extra_borg_options.split(' ')) if extra_borg_options else ())
+ (repository,)
Review

Shouldn't this be an or rather than an and, so --files works at multiple log levels? And for prune, is the idea that the file listing should still be enabled at INFO, rather than demoting it to DEBUG like with create?

Shouldn't this be an `or` rather than an `and`, so `--files` works at multiple log levels? And for `prune`, is the idea that the file listing should still be enabled at `INFO`, rather than demoting it to `DEBUG` like with `create`?

My intention was to only show the file listing at INFO and not at WARNING level. This is related to the old influence of the --stats option to change the logging level of the borg output. Since now --statshas no influence on the borg output level anymore, I could simplify the condition to if files.

To make the code a bit more consistent I moved the --listout of extra DEBUG line similar as in create.py:

    + (('--list',) if files or logger.isEnabledFor(logging.DEBUG) else ())
    + (('--debug', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ())
My intention was to only show the file listing at INFO and not at WARNING level. This is related to the old influence of the `--stats` option to change the logging level of the borg output. Since now `--stats`has no influence on the borg output level anymore, I could simplify the condition to `if files`. To make the code a bit more consistent I moved the `--list`out of extra DEBUG line similar as in `create.py`: ``` + (('--list',) if files or logger.isEnabledFor(logging.DEBUG) else ()) + (('--debug', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ()) ```
Review

Looks good, thanks!

Looks good, thanks!
)
execute_command(
full_command,
output_log_level=logging.WARNING if stats else logging.INFO,
error_on_warnings=False,
)
execute_command(full_command, output_log_level=logging.INFO, error_on_warnings=False)

View File

@ -237,6 +237,13 @@ def parse_arguments(*unparsed_arguments):
action='store_true',
help='Display statistics of archive',
)
prune_group.add_argument(
'--files',
dest='files',
default=False,
action='store_true',
help='Show file details and stats at verbosity 1',
)
prune_group.add_argument('-h', '--help', action='help', help='Show this help message and exit')
create_parser = subparsers.add_parser(
@ -261,6 +268,13 @@ def parse_arguments(*unparsed_arguments):
action='store_true',
help='Display statistics of archive',
)
create_group.add_argument(
'--files',
dest='files',
default=False,
action='store_true',
help='Show file details and stats at verbosity 1',
)
create_group.add_argument(
'--json', dest='json', default=False, action='store_true', help='Output results as JSON'
)

View File

@ -212,6 +212,7 @@ def run_actions(
local_path=local_path,
remote_path=remote_path,
stats=arguments['prune'].stats,
files=arguments['prune'].files,
)
if 'create' in arguments:
logger.info('{}: Creating archive{}'.format(repository, dry_run_label))
@ -225,6 +226,7 @@ def run_actions(
progress=arguments['create'].progress,
stats=arguments['create'].stats,
json=arguments['create'].json,
files=arguments['create'].files,
)
if json_output:
yield json.loads(json_output)

View File

@ -98,12 +98,14 @@ def test_parse_arguments_with_no_actions_defaults_to_all_actions_enabled():
def test_parse_arguments_with_no_actions_passes_argument_to_relevant_actions():
flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default'])
arguments = module.parse_arguments('--stats')
arguments = module.parse_arguments('--stats', '--files')
assert 'prune' in arguments
assert arguments['prune'].stats
assert arguments['prune'].files
assert 'create' in arguments
assert arguments['create'].stats
assert arguments['create'].files
assert 'check' in arguments
@ -423,6 +425,25 @@ def test_parse_arguments_with_stats_flag_but_no_create_or_prune_flag_raises_valu
module.parse_arguments('--stats', 'list')
def test_parse_arguments_with_files_and_create_flags_does_not_raise():
flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default'])
module.parse_arguments('--files', 'create', 'list')
def test_parse_arguments_with_files_and_prune_flags_does_not_raise():
flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default'])
module.parse_arguments('--files', 'prune', 'list')
def test_parse_arguments_with_files_flag_but_no_create_or_prune_or_restore_flag_raises_value_error():
flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default'])
with pytest.raises(SystemExit):
module.parse_arguments('--files', 'list')
def test_parse_arguments_allows_json_with_list_or_info():
flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default'])

View File

@ -295,7 +295,7 @@ def test_create_archive_with_log_info_calls_borg_with_info_parameter():
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
flexmock(module).should_receive('execute_command').with_args(
('borg', 'create', '--list', '--filter', 'AME-', '--info', '--stats') + ARCHIVE_WITH_PATHS,
('borg', 'create', '--info') + ARCHIVE_WITH_PATHS,
output_log_level=logging.INFO,
error_on_warnings=False,
)
@ -432,8 +432,7 @@ def test_create_archive_with_dry_run_and_log_info_calls_borg_without_stats_param
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
flexmock(module).should_receive('execute_command').with_args(
('borg', 'create', '--list', '--filter', 'AME-', '--info', '--dry-run')
+ ARCHIVE_WITH_PATHS,
('borg', 'create', '--info', '--dry-run') + ARCHIVE_WITH_PATHS,
output_log_level=logging.INFO,
error_on_warnings=False,
)
@ -850,7 +849,7 @@ def test_create_archive_with_stats_calls_borg_with_stats_parameter():
flexmock(module).should_receive('_make_exclude_flags').and_return(())
flexmock(module).should_receive('execute_command').with_args(
('borg', 'create', '--stats') + ARCHIVE_WITH_PATHS,
output_log_level=logging.WARNING,
output_log_level=logging.INFO,
error_on_warnings=False,
)
@ -875,8 +874,7 @@ def test_create_archive_with_progress_and_log_info_calls_borg_with_progress_para
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
flexmock(module).should_receive('execute_command_without_capture').with_args(
('borg', 'create', '--info', '--stats', '--progress') + ARCHIVE_WITH_PATHS,
error_on_warnings=False,
('borg', 'create', '--info', '--progress') + ARCHIVE_WITH_PATHS, error_on_warnings=False
)
insert_logging_mock(logging.INFO)

View File

@ -75,9 +75,7 @@ def test_prune_archives_with_log_info_calls_borg_with_info_parameter():
flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return(
BASE_PRUNE_FLAGS
)
insert_execute_command_mock(
PRUNE_COMMAND + ('--stats', '--info', '--list', 'repo'), logging.INFO
)
insert_execute_command_mock(PRUNE_COMMAND + ('--info', 'repo'), logging.INFO)
insert_logging_mock(logging.INFO)
module.prune_archives(
@ -91,7 +89,7 @@ def test_prune_archives_with_log_debug_calls_borg_with_debug_parameter():
BASE_PRUNE_FLAGS
)
insert_execute_command_mock(
PRUNE_COMMAND + ('--stats', '--debug', '--list', '--show-rc', 'repo'), logging.INFO
PRUNE_COMMAND + ('--stats', '--list', '--debug', '--show-rc', 'repo'), logging.INFO
)
insert_logging_mock(logging.DEBUG)
@ -149,7 +147,7 @@ def test_prune_archives_with_stats_calls_borg_with_stats_parameter():
flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return(
BASE_PRUNE_FLAGS
)
insert_execute_command_mock(PRUNE_COMMAND + ('--stats', 'repo'), logging.WARNING)
insert_execute_command_mock(PRUNE_COMMAND + ('--stats', 'repo'), logging.INFO)
module.prune_archives(
dry_run=False,