Skip to content
This repository has been archived by the owner on Feb 3, 2022. It is now read-only.

Only clean up tmp file if rename was successful #30

Closed
punkstar opened this issue Jan 21, 2016 · 2 comments
Closed

Only clean up tmp file if rename was successful #30

punkstar opened this issue Jan 21, 2016 · 2 comments
Assignees
@bobbyshaw

Comments

@punkstar
Copy link
Contributor

There already seems to be a check for this in checking the return type for rename (Meanbee\Magedbm\Command\GetCommand::backupMove) but I've seen a warning emitted and rename returning true when using meanbee/docker-magento-tools.

The fix would be to perform is_writable check on $newFilename directory first.
@bobbyshaw
Copy link
Member

How about a file_exists check after the rename has been done for a more confident check? Or both, if I want to be really verbose with exceptions thrown.

@bobbyshaw
Copy link
Member

I've added a PR for this but it would be good to know what the warning was as I haven't been able to reproduce by removing permissions. As I don't actually clean up the tmp directory in this branch of code so I'm not sure where your file would have gone.

@bobbyshaw bobbyshaw self-assigned this Mar 14, 2016
bobbyshaw added a commit that referenced this issue Mar 16, 2016
@bobbyshaw
Merge pull request #31 from meanbee/30-improve-check-for-file-move-su…
e4cb77e 
…ccess

Fixes #30: Check directory is writable before moving downloaded file.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bobbyshaw bobbyshaw

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@punkstar @bobbyshaw