Skip to content
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

fix non-writable directory not identified as such #54

Merged
merged 1 commit into from Apr 5, 2019

Conversation

@nickdavis
Copy link
Contributor

commented Apr 5, 2019

fixes #52

will add tests in a subsequent commit

@nickdavis nickdavis requested a review from wp-cli/committers as a code owner Apr 5, 2019

@schlessera schlessera added the bug label Apr 5, 2019

@danielbachhuber
Copy link
Member

left a comment

Thanks @nickdavis ! I don't think tests are necessary in this case, because it would require creating an unwritable path in the test suite (which is a bit of a pain to deal with)

@danielbachhuber danielbachhuber added this to the 2.0.2 milestone Apr 5, 2019

@danielbachhuber danielbachhuber merged commit a8b3a26 into wp-cli:master Apr 5, 2019

2 checks passed

DEP All dependencies are resolved
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ShashwatMittal

This comment has been minimized.

Copy link

commented Apr 8, 2019

@danielbachhuber I see that the issue is fixed. But the error handling is already there in the Export Command here ->

throw new WP_Export_Exception( sprintf( __( 'WP Export: error opening %s for writing.' ), $file_path ) );

But I think it's not working properly. Is that supposed to be like this or is there any better way?

@danielbachhuber

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

@ShashwatMittal I think what would happen is that start_new_file() would fail, skip writing the current file, and proceed to the next file. With this pull request, the entire process fails when the directory isn't writable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.