-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
ls: Add JSON output support for restic ls cmd #1953
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1953 +/- ##
==========================================
- Coverage 50.5% 46.16% -4.34%
==========================================
Files 170 170
Lines 13266 13304 +38
==========================================
- Hits 6700 6142 -558
- Misses 5559 6203 +644
+ Partials 1007 959 -48
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good! Thanks for working on it during your travel. It's an elegant solution, especially from a developer who's new to Go.
One thing of note is that currently the ls
output is streamed to stdout, printing matches as it finds them, without buffering the whole list in memory. This approach, for the JSON output, does buffer each item from all snapshots in memory before encoding them. Most of the time this won't be a problem, but I can imagine memory bloat when a listing has tens of thousands (or more) items.
I'm inclined to accept this change, though, since I think it's a really good one, and leave a change to streaming JSON output "as-needed". Heck, I might even be willing to build that on top of this later.
Hey, thank you very much for your contribution! I have not looked at the code yet, but I think it'd be good to not print anything about the snapshot (programs will probably have that information already, or they can call |
Really looking forward to this! I built a backup GUI (served via GraphQL API), and it currently relies on some poorly-performing and ugly JS regexes to parse the results of |
@frankdugan3 Cool, do you have your GUI anywhere? I'm interested to see it! |
@mholt As of right now it's pretty integrated into a much larger ERP app I'm working on for my employer. But I've been thinking about the best way to release a more generalized version for distribution. Currently torn between making a plugin/GraphQL middleware (in JS) just to help with getting API access, or making a more general GUI webapp binary (in Go). |
@frankdugan3 Gotcha -- that sounds like a fun project. (I'm working on one too) I've really enjoyed making restic visually accessible. I think this PR will be a great benefit for that. |
@mholt Wow... That looks nice! Well, in that case I’ll sub to updates and maybe just publish a GraphQL lib! |
Hm, now I see why we need the info about the snapshots: the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rebased the commits on the latest master branch and improved the changelog entry a bit, but apart from that it's great! Thanks for your contribution!
ls: Add JSON output support for restic ls cmd
I have a couple ideas for this, since I'm still interested in streaming the results out:
What do you think of either of those options for streaming?
Thanks! No doubt there will be users who find a GraphQL lib useful. I saw your reply on Twitter too, we'd be happy to have you help us test it out -- we'll be ready in a couple of weeks! |
I thought that you'd be interested in it ;) I think it'd be fine to have a stream of json objects, one per line, starting with a snapshot object, then all nodes, then maybe another snapshot object. If you want to work on that, please go ahead and let's discuss the details in the next PR or issue. |
What is the purpose of this change? What does it change?
This PR adds support for JSON output for the restic ls command, using the existing global --json flag.
output truncated of :
./restic ls 1977f34d /doc/_static --json
Was the change discussed in an issue or in the forum before?
Previous PR : #1939
Checklist
changelog/unreleased/
that describes the changes for our users (template here)gofmt
on the code in all commits