-
Notifications
You must be signed in to change notification settings - Fork 59
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
Various improvements to the exported files controller #292
base: master
Are you sure you want to change the base?
Conversation
@@ -62,13 +62,11 @@ | |||
|
|||
#support paths | |||
get "utils(.:format)", controller: "support", action: "index", as: "utils" | |||
get "utils/files/:id(.:format)", controller: "support", action: "destroy", constraints: { id: /[^\/]+/ }, as: "utils_files" | |||
delete "utils/files/:id(.:format)", controller: "support", action: "destroy", constraints: { id: /[^\/]+/ }, as: "utils_files" |
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.
Metrics/LineLength: Line is too long. 128/100
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.
does this require some change in the cli?
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.
No, it's not part of the CLI yet.
It's really horrible to put the files in the db directory, so don't do that. Also properly clean up files at the end.
While not strictly needed, it's slightly better in order to avoid non-crowbar owned files in the rails app tmp directory. We use a chown at the end, of course, but if the rails app is stopped in the middle, then the tmp directory contains files that can't be removed. Using a proper temporary directory helps (since it may be cleaned up on reboot, or by scripts).
They were probably used a long time ago...
This is the same as the title of the page.
+1 |
exports = Dir.glob(Rails.root.join("db", "*.json").to_s) | ||
cmd = ["tar", "-czf", Rails.root.join("tmp", filename).to_s, *exports] | ||
tmpdir = Dir.mktmpdir | ||
tmpdirpath = Pathname.new(tmpdir) |
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.
MINOR: i would call it tmpdir_path
for consistency
or as an alternative just use tmpdir = Dir.mktmpdir(Pathname.new(tmpdir))
and reference it with .to_s
where needed
just unsure about the cli part (don't know if we even offer it) +1 |
No description provided.