analyze: add --json output, fixes #9992 - #10001
Open
MannXo wants to merge 2 commits into
Open
Conversation
Every other status-type command (info, repo-info, repo-list, list, diff, prune) can emit JSON, so tooling does not have to scrape their text. borg analyze was the odd one out, although its numbers are exactly what monitoring wants. --json emits the numbers the text report is rendered from, as raw byte values, for the default mode (dedup_size, hotspots) as well as for --by-name (by_name). The compression factor is left out: it is stored_size / source_size, and "n/a" is not a useful JSON value. To keep one source of truth, the analysis methods now return their numbers and the printing moved into report_*() methods that format them. The text output is unchanged, byte for byte. hotspots is null rather than empty when fewer than two archives matched: the hot spots were then not computed at all, which is different from having computed them and found nothing.
Member
|
Opus being dramatic as usual. Sabotage! :-) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10001 +/- ##
==========================================
+ Coverage 85.88% 85.90% +0.02%
==========================================
Files 95 95
Lines 17088 17116 +28
Branches 2616 2622 +6
==========================================
+ Hits 14676 14704 +28
+ Misses 1674 1673 -1
- Partials 738 739 +1 ☔ View full report in Codecov by Harness. |
Member
|
Windows CI is failing due to path normalization. |
The test rebuilt the expected hot-spot path from the input directory, stripping a leading slash. Archived paths are normalized, and on Windows that also drops the drive colon (C:\Users -> C/Users), so the expectation read D:/a/... where borg had stored D/a/.... Assert the size of the input directory's hot spot by path suffix, like the text-report test above already does, and check the paths against what the text report prints instead of rebuilding them.
Author
reflected |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
--jsontoborg analyze, as proposed in #9992.The numbers are emitted as raw byte values, for the default mode as well as for
--by-name. Output goes to stdout as one JSON object, like the other--jsoncommands.(Reformatted to fit: the real output puts one key per line. The numbers are from the fixture repository described under Testing, with the repository id and paths genericized.)
Decisions worth a second opinion
stored / source, but in JSON that is derivable, and the "n/a" the text uses where there is no source size is not a useful JSON value. Say the word and I will add it.hotspotsisnull, not[], when fewer than two archives matched. The hot spots were then not computed at all, which is different from having computed them and found no churn.--by-nameomits the key entirely, as it does no hot-spot analysis.exclusiveis absent whenwhole_repositoryis true. That mirrors the text report, which suppresses the line because every referenced chunk is then trivially exclusive to the set.missing_chunksis included although the text report only logs it as a warning. Monitoring wants it, and dropping it would leave a JSON consumer unable to see that the numbers are based on an incomplete index.source_sizeandstored_size.Refactor
The analysis methods now return their numbers, and printing moved into
report_dedup_size(),report_by_name()andreport_hotspots(), so the text report and the JSON come from one source of truth rather than two.The text output is unchanged, byte for byte. I checked that. I built a fixture repository (2 archives named
alpha, 1 namedbeta, sharing one file, with churn in a subdirectory), capturedanalyzein six variants such as: whole repository, a matching set, a single archive,--by-name, the--by-name+ filter error, and the no-match error — onmasterand on this branch, and diffed the captured stdout and stderr:Testing
4 new tests in
analyze_cmd_test.py, covering the default mode, the whole-repository case, hot spots (both computed and not computed), and--by-name.Exactly the 4 new tests, no new skips. The skips are the fakeroot, FUSE and Linux-only ones; I ran this on macOS, so CI covers those.
Each new test was checked by reverting the behaviour it covers, one sabotage at a time, and confirming the intended test fails and then passes again:
exclusive, also for the whole repositorytest_analyze_json_whole_repositoryhotspots = []instead ofnullwhen not computedtest_analyze_json_hotspotstest_analyze_json(+ 6 text-report tests)--by-namerows smallest firsttest_analyze_json_by_nametest_analyze_by_name,test_analyze_json_by_nametest_analyze_by_name,test_analyze_json_by_nametest_analyze_json, and 2 more--by-nameresult under thededup_sizekeytest_analyze_json_by_nameThe dedup-size test does not hardcode the compression-dependent stored sizes. It asserts that both columns of the corresponding text row are
format_file_size()of the JSON values, which pins the "same numbers the text report is rendered from" property directly.black --check,ruff checkandpre-commit runare clean on the changed files.cd docs && make htmlbuilds, and the new:ref:resolves (href="../internals/frontends.html#json-output").Docs
docs/internals/frontends.rstgains an Archive Analysis subsection describing every key, plus an example. The example is real captured output with the repository id and paths genericized.:ref:to that section, with a plain-text substitution inrst_plain_text_referencessoborg analyze --helprenders it as a sentence rather than leaking markup.docs/usage/analyze.rst.incis regenerated (python scripts/make.py build_usage).I did not regenerate
docs/man/borg-analyze.1. It is stale from an earlier change (t still documents only the hot spots and none of the dedup-size report) so regenerating it would fold an unrelated docs catch-up into this PR. Happy to add it here, or in a separate commit, whichever you prefer.I also have not touched
CHANGES.rst, since recent feature commits do not.Claude Code assisted with this change(Opus 5). I verified every change including the byte-for-byte text-output comparison, the per-test sabotage table and the docs build above are the evidence, and the JSON examples are captured output, not written by hand.