add support for --list --json
#74
No reviewers
Labels
No Label
bug
data loss
design finalized
good first issue
new feature area
question / support
security
waiting for response
No Milestone
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: borgmatic-collective/borgmatic#74
Loading…
Reference in New Issue
No description provided.
Delete Branch ":list-json"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
as discussed, here's a first try at supporting json output from borgmatic for the
borg list
command.Outputed json is wrapped in an array to handle multi-repository cases.
Note I had to use
tempfile.TemporaryFile()
sincesuprocess.check_call()
does not supportio.StringIO()
for itsstdout
parameter.Finally got a chance to look at this. Thanks for making this enhancement! Here's some feedback:
subprocess.check_call()
with a temporary file, you should just be able to switch tosubprocess.check_output()
and get the output you're looking for as a return value of that call. Then you won't need that temp file.list_archives()
to telegraph the contract change: That sometimes it returns an output value and sometimes it doesn't, based on the value of thejson
parameter.list_archives()
unconditionally return the output value (regardless of JSON or not), and make the caller responsible for dealing with it? That might simplify the contract, since the caller is having to deal with the output anyway.run_coniguration()
function is so difficult to test, as it would be nice to have some coverage around the outer JSON list assembly and output. I'll have to think about how to make this function more testable.. If you have any ideas, let me know.Also: Reminder to update the AUTHORS file if you like!
MR updated based on your feedback.
I also gave a try at testing the JSON list assembly bit in
run_configuration()
.And it appears I'm already in the AUTHORS list ;)
Awesome, thanks for incorporating feedback and doing the refactoring to support test coverage. Looks good.. I'll merge this now. There are a few (totally undocumented) coding convention issues around whitespace, but I can fix those after the merge.
Oh, and about the AUTHORS file: It just mentions you in regards to one particular feature, so feel free to take credit for additional features there if you like. (Or just generalize the description if that sounds too high-maintenance.) I just want to make sure credit is given appropriately!
Released in borgmatic 1.2.1!