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
Unknown Object (File)
Mon, Dec 16, 2:38 PM
Unknown Object (File)
Sun, Dec 15, 6:08 PM
Unknown Object (File)
Fri, Dec 13, 9:02 PM
Unknown Object (File)
Sun, Dec 8, 2:25 PM
Unknown Object (File)
Sat, Dec 7, 8:42 PM
Unknown Object (File)
Fri, Dec 6, 1:51 AM
Unknown Object (File)
Fri, Dec 6, 12:54 AM
Unknown Object (File)
Thu, Dec 5, 11:54 PM
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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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.