add support for 'diff --json-lines', available in borg >= 1.1.16 #909
Conversation
|
Nice! |
|
yes, hopefully my PR on borg will be completed soon. I think they're trying to get it in for the 1.1.16 release. |
rblenis
force-pushed
the
issue_#901
branch
2 times, most recently
from
0ac07fc to
27e9986
Compare
rblenis
force-pushed
the
issue_#901
branch
2 times, most recently
from
a5f81a4 to
834e627
Compare
rblenis
changed the title
|
borg 1.1.16 released with 'diff --json-lines' support. |
|
@m3nu, should the test workflow also use several versions of borg, this way some features which behave differently depending on borg version (like this feature), will get moer thoroughly tested? |
|
Most calls to Borg are mocked and it just uses pre-defined json output files in |
|
Yeah, let's get this feature out. I was waiting with merges until the Debian package is frozen. That should be done by now. |
|
just realized, I didn't look at the test code for the diff command. I guess it is assuming pre 1.1.16 and just testing the non-json-lines output. I'll look at adding some test code for the json-lines, but not sure if I can get to it until this weekend. |
|
That would be perfect. Thanks! |
fixes issue borgbase#901 (diff not handling CR/LF in filenames)
|
I added some tests, copying the non-json-lines test. I also made small changes to report the changes in the tree dialog to match the non-json-lines. Later, I may look at improving the difference info displayed in the tree, especially when multiple types of changes occur to the same item. |
|
Looks all good to me. This is ready to merge? |
|
yes it's ready. |
| def pretty_bytes(size): | ||
| """from https://stackoverflow.com/questions/12523586/ | ||
| python-format-size-application-converting-b-to-kb-mb-gb-tb/37423778""" | ||
| def pretty_bytes(size, metric=True, sign=False, precision=1): |
There was a problem hiding this comment.
Nice improvements to pretty_bytes(). But you don't use the new metric and sign args anywhere?
There was a problem hiding this comment.
I also removed the SO reference, since the function substantially changed.
There was a problem hiding this comment.
In an earlier version of my changes, I was using some of the enhancements to pretty_bytes, but after adding the test code and realizing the displayed tree did not match exactly with the non-json-lines version, I didn't use it, and forgot to revert those changes. There are two 'bug-fixes', but I don't think either one would be triggered. 1. better handling of something larger than 1000TB - my version with the loop index check will at least attempt to print the number using 'TB' units, instead of NaN (due to array index exception). 2. better handling of negative 'size' - print correct value with appropriate units instead of using base/unscaled units ('B'). Currently, I think the tree only shows the '+' change in file content, and ignores the '-' change in file content (which I plan to investigate and submit PR), so # 2 probably isn't currently triggered either. And then a 3rd 'issue', is when a number is close to the threshold of using the next larger units, but doesn't use them, but then due to rounding prints as '1000' ... Example: size = 999.95 KB -- it is not large enough to go through the loop twice and use 'MB' units, but when printed, it is rounded up and prints as '1000.0 KB' instead of '1.0 MB'. It seems like the preferred display would limit the significand to 4 total digits and not sometimes round up and display 5 digits.
There was a problem hiding this comment.
I could split off the pretty_bytes changes as a separate PR and indicate the issues it addresses.
There was a problem hiding this comment.
Nah, it's fine. The new args might be useful in the future and it fixes some issues as you say.
src/vorta/borg/diff.py
Outdated
| ret['archive_name_newer'] = archive_name_2 | ||
| ret['json_lines'] = False | ||
| if borg_compat.check('DIFF_JSON_LINES'): | ||
| ret['cmd'].insert(4, '--json-lines') |
There was a problem hiding this comment.
This is dangerous because the hard-coded position is often overlooked when someone makes changes later. Minor change to get rid of it.
|
Few administrative edits. |
fixes issue #901 (diff not handling CR/LF in filenames)