New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backup: Support --json flag by streaming JSON to report progress #1944

Merged
merged 2 commits into from Feb 10, 2019

Conversation

Projects
None yet
4 participants
@mholt
Copy link
Contributor

mholt commented Aug 11, 2018

What is the purpose of this change? What does it change?

restic backup now supports --json so that it can stream JSON output to update the progress of the backup, making it better for non-interactive, but supervised, use.

Currently, the basic updates are sent as JSON and seem to be working, but I am still transitioning the copy of ui.Backup to the new jsonstatus.Backup type. I will iterate on this.

Was the change discussed in an issue or in the forum before?

Closes #1937

Checklist

  • I have read the Contribution Guidelines
  • I have added tests for all changes in this PR
  • I have added documentation for the changes (in the manual)
  • There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
  • I have run gofmt on the code in all commits
  • All commit messages are formatted in the same style as the other commits in the repo
  • I'm done, this Pull Request is ready for review

@mholt mholt changed the title backup: Support --json flag by streaming JSON to report progress backup: Support --json flag by streaming JSON to report progress (WIP) Aug 11, 2018

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Aug 11, 2018

Codecov Report

Merging #1944 into master will decrease coverage by 4.22%.
The diff coverage is 65.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1944      +/-   ##
==========================================
- Coverage   50.76%   46.54%   -4.23%     
==========================================
  Files         176      176              
  Lines       14188    14216      +28     
==========================================
- Hits         7203     6617     -586     
- Misses       5933     6603     +670     
+ Partials     1052      996      -56
Impacted Files Coverage Δ
internal/ui/backup.go 0% <0%> (ø) ⬆️
cmd/restic/global.go 27.27% <0%> (ø) ⬆️
cmd/restic/cmd_backup.go 44.76% <73.8%> (+3.58%) ⬆️
internal/backend/b2/b2.go 0% <0%> (-80.69%) ⬇️
internal/backend/swift/swift.go 0% <0%> (-78.45%) ⬇️
internal/backend/gs/gs.go 0% <0%> (-74%) ⬇️
internal/backend/azure/azure.go 0% <0%> (-69.46%) ⬇️
internal/backend/swift/config.go 36.95% <0%> (-54.35%) ⬇️
internal/checker/checker.go 64.54% <0%> (-3.87%) ⬇️
internal/archiver/blob_saver.go 92.59% <0%> (-2.47%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1107eef...6cf1348. Read the comment docs.

@mholt

This comment has been minimized.

Copy link
Contributor Author

mholt commented Aug 13, 2018

Alrighty -- I've finished implementing restic --json backup so that all the output is JSON-formatted. It also respects verbosity levels. Feel free to give it a spin!

Again, I don't feel this is the cleanest solution but I don't know a better way right now. Happy to have your thoughts or modifications if you have any ideas. :)

For the purposes of a convenient review, I've left in (comments) the original, human-formatted outputs so that you can compare what was being printed to the terminal and what is being populated in the JSON fields. They should be the same, sans fancy formatting. I can remove those comments before a final merge.

Although functional, the code is not really in its final state, so I expect to iterate a bit. Let me know what you think!

@@ -355,7 +355,7 @@ func OpenRepository(opts GlobalOptions) (*repository.Repository, error) {
return nil, err
}

if stdoutIsTerminal() {
if stdoutIsTerminal() && !opts.JSON {

This comment has been minimized.

@mholt

mholt Aug 19, 2018

Author Contributor

Just realized I made a similar, but slightly different variant of this change over at #1962: https://github.com/restic/restic/pull/1962/files#diff-952c97fd56de92e641e61b3b3aa881dbR363

@mholt

This comment has been minimized.

Copy link
Contributor Author

mholt commented Oct 4, 2018

Resolved the merge conflict.

Would anyone else like to try this out? I think it's pretty cool. I just don't love the code that does it. It'd be great to get another set of eyes on the code to get ready to merge this.

@Mebus

This comment has been minimized.

Copy link

Mebus commented Nov 23, 2018

Hi Matt,

I just tested it. This looks great!

Can you please also include the ID of the created snapshot in the summary? This would be useful for a GUI.

→ restic -r /tmp/thebackup6 --json backup changelog
found 7 old cache directories in /home/mebus/.cache/restic, pass --cleanup-cache to remove them
{"message_type":"status","percent_done":0,"total_files":1,"total_bytes":364}
{"message_type":"status","percent_done":0.01380618243883937,"total_files":83,"files_done":1,"total_bytes":26365,"bytes_done":364,"current_files":["/changelog/0.6.0_2017-05-29/issue-953","/changelog/0.6.0_2017-05-29/issue-965"]}
{"message_type":"status","percent_done":0.14339015151515153,"total_files":140,"files_done":22,"total_bytes":52800,"bytes_done":7571,"current_files":["/changelog/0.7.2_2017-09-13/issue-1132","/changelog/0.7.2_2017-09-13/issue-1167"]}
{"message_type":"status","percent_done":0.34890151515151513,"total_files":140,"files_done":59,"total_bytes":52800,"bytes_done":18422,"current_files":["/changelog/0.8.1_2017-12-27/issue-1457"]}
{"message_type":"status","percent_done":1,"total_files":140,"files_done":140,"total_bytes":52800,"bytes_done":52800}
{"message_type":"summary","files_new":140,"files_changed":0,"files_unmodified":0,"dirs_new":0,"dirs_changed":0,"dirs_unmodified":0,"data_blobs":140,"tree_blobs":1,"data_added":53295,"total_files_processed":140,"total_bytes_processed":52800,"total_duration":0.7903416}

Thanks

Mebus

@mholt

This comment has been minimized.

Copy link
Contributor Author

mholt commented Nov 24, 2018

Sure thing @Mebus! Thanks for testing it. I'll revisit this PR shortly.

@fd0 fd0 force-pushed the mholt:jsonprogress branch 3 times, most recently from 2a89a3a to c1e337c Dec 1, 2018

@mholt mholt changed the title backup: Support --json flag by streaming JSON to report progress (WIP) backup: Support --json flag by streaming JSON to report progress Dec 3, 2018

@mholt mholt referenced this pull request Jan 7, 2019

Merged

Use processed bytes for summary #2138

3 of 3 tasks complete

@mholt mholt requested a review from fd0 Jan 7, 2019

@mholt

This comment has been minimized.

Copy link
Contributor Author

mholt commented Jan 16, 2019

This PR is ready for approval and merge

@mholt

This comment has been minimized.

Copy link
Contributor Author

mholt commented Feb 6, 2019

@fd0 I know you're busy, but can you just click the Merge button? 😅 This is ready to go. 👍

@fd0

fd0 approved these changes Feb 10, 2019

Copy link
Member

fd0 left a comment

LGTM!

@fd0 fd0 force-pushed the mholt:jsonprogress branch from cbaecf3 to 6cf1348 Feb 10, 2019

@fd0

This comment has been minimized.

Copy link
Member

fd0 commented Feb 10, 2019

I've rebased and resolved the merge conflict (and removed the merge of the master branch), and I'll merge this as soon as the CI tests complete :)

@fd0 fd0 merged commit 6cf1348 into restic:master Feb 10, 2019

2 checks passed

Hound No violations found. Woof!
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

fd0 added a commit that referenced this pull request Feb 10, 2019

@mholt mholt deleted the mholt:jsonprogress branch Feb 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment