borgmatic-collective/borgmatic
borgmatic-collective
/
borgmatic
15
57
Fork 41
Code Issues 55 Pull Requests 1 Actions Packages Releases 121 Activity

Implement json output for create_archive and pretty print json output. #97

Closed
floli wants to merge 2 commits from (deleted):master into main
pull from: (deleted):master
merge into: borgmatic-collective:main
Conversation 15 Commits 2 Files Changed 2 +9 -3
2 changed files with 9 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
borgmatic/borg/create.py
View File

@ -157,4 +157,5 @@ def create_archive(
)
logger.debug(' '.join(full_command))
subprocess.check_call(full_command)
output = subprocess.check_output(full_command)
witten commented 2018-09-30 18:26:52 +00:00
Review
Copy Link

Does this approach (check_output() instead of check_call()) have the side effect of delaying any borg create output until the command fully runs and exits? Do you see that as a usability problem, or not something to worry about? If you want to address this, one idea is to conditionally run check_call() or check_output() based on whether args.json is True.

Note that I don't feel strongly about this either way.. Just wanted to bring it up and get your take.

Does this approach (`check_output()` instead of `check_call()`) have the side effect of delaying any `borg create` output until the command fully runs and exits? Do you see that as a usability problem, or not something to worry about? If you want to address this, one idea is to conditionally run `check_call()` or `check_output()` based on whether `args.json` is `True`. Note that I don't feel strongly about this either way.. Just wanted to bring it up and get your take.
return output.decode() if output is not None else None
witten commented 2018-09-30 17:23:56 +00:00
Review
Copy Link

Instead of conditionally decoding after the fact, I think you should be able to pass the desired encoding directly to check_output(). Example:

     output = subprocess.check_output(full_command, encoding=sys.stdout.encoding)
Instead of conditionally decoding after the fact, I think you should be able to pass the desired encoding directly to `check_output()`. Example: ``` output = subprocess.check_output(full_command, encoding=sys.stdout.encoding) ```

9
borgmatic/commands/borgmatic.py
View File

@ -155,7 +155,7 @@ def _run_commands(args, consistency, local_path, location, remote_path, retentio
unexpanded_repository,
)
if args.json:
sys.stdout.write(json.dumps(json_results))
sys.stdout.write(json.dumps(json_results, indent=2))
def _run_commands_on_repository(
@ -176,14 +176,19 @@ def _run_commands_on_repository(
)
if args.create:
logger.info('{}: Creating archive{}'.format(repository, dry_run_label))
borg_create.create_archive(
output = borg_create.create_archive(
args.dry_run,
repository,
location,
storage,
local_path=local_path,
remote_path=remote_path,
json=args.json,
)
if args.json:
json_results.append(json.loads(output))
else:
sys.stdout.write(output)
witten commented 2018-09-30 18:24:07 +00:00
Review
Copy Link

Given the repetition of this pattern a few times in this function, you could DRY this up by factoring it out into a common function. Example imagined invocation:

display_output(output, args.json)
Given the repetition of this pattern a few times in this function, you could DRY this up by factoring it out into a common function. Example imagined invocation: ``` display_output(output, args.json) ````
if args.check:
logger.info('{}: Running consistency checks'.format(repository))
borg_check.check_archives(