Page MenuHomePhabricator

Provide an "--output" mode for "bin/storage dump" with better error handling
ClosedPublic

Authored by epriestley on Aug 17 2016, 1:35 PM.
Tags
None
Referenced Files
F14057992: D16407.diff
Sun, Nov 17, 8:30 AM
F14055643: D16407.id39459.diff
Sat, Nov 16, 2:11 PM
F14055642: D16407.id39460.diff
Sat, Nov 16, 2:11 PM
F14055641: D16407.id39456.diff
Sat, Nov 16, 2:11 PM
F14055640: D16407.id39455.diff
Sat, Nov 16, 2:11 PM
F13991747: D16407.id39456.diff
Tue, Oct 22, 12:34 PM
F13962819: D16407.id39460.diff
Oct 15 2024, 12:31 PM
F13961657: D16407.id.diff
Oct 15 2024, 5:50 AM
Subscribers
None

Details

Summary

Ref T6996. If you do this kind of thing in the shell, you don't get a good error by default if the dump command fails:

$ bin/storage dump | gzip > output.sql.gz

This can be worked around with some elaborate bash tricks, but they're really clunky and uninintuitive.

We also need to do this in several places (while writing backups; while performing exports), and I don't want to copy clunky bash tricks all over the codebase.

Instead, provide --output and --compress flags which just do this processing inside bin/storage dump. It will fail appropriately if any of the underlying operations fail. This also makes the write a little safer (refuses to overwrite) and the code more reusable.

Test Plan
  • Did three dumps, with no flags, --output, and --output --compress.
  • Verified all three took similar amounts of time and were identical except for "Date Exported" timestamps in comments (except that the compressed one was compressed).
  • Used gunzip to examine the compressed one, verified it was really compressed.
  • Faked a write error, saw properly command behavior (clean up file + exit with error).

Diff Detail

Repository
rP Phabricator
Branch
dumpx
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 13335
Build 17105: Run Core Tests
Build 17104: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Provide an "--output" mode for "bin/storage dump" with better error handling.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
  • Allow overwrites if the file exists but is empty. This makes interaction with TempFile a little cleaner.
chad edited edge metadata.
This revision is now accepted and ready to land.Aug 17 2016, 1:48 PM
epriestley added a child revision: Restricted Differential Revision.Aug 17 2016, 2:05 PM
epriestley edited edge metadata.
  • Actually, add an explicit "--overwrite" flag instead of having weird "overwrite empty files" semantics.
  • Slightly better error message to suggest using "--overwrite".
This revision was automatically updated to reflect the committed changes.