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

FileSystem driver now tries to create cache folder before throwing InvalidArgumentException #66

Merged
merged 1 commit into from Mar 30, 2013

Conversation

lolautruche
Copy link
Contributor

Hello

Fixes tedious/TedivmStashBundle#15

This is a very small change allowing FileSystem driver to try to create the cache folder if it doesn't already exist. If folder creation fails, then the exception is thrown.

Note the usage of the silent operator (@). It's here only to avoid a verbose warning if folder creation fails since we want to throw an exception.

@@ -385,7 +385,10 @@ protected function checkFileSystemPermissions()
}

if(!is_dir($this->cachePath)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is better:

if (!is_dir($this->cachepath) && !mkdir($this->cachePath, $this->dirPermissions, true)) {
    throw new InvalidArgumentException(sprintf('Failed to create cache path %s', $this->cachePath));
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right 😃 . Fixed

@andrerom andrerom mentioned this pull request Mar 8, 2013
@lolautruche
Copy link
Contributor Author

Note : Travis is not failing because of this patch, but because of Memached extension installation (probably because of this : http://about.travis-ci.org/blog/2013-03-08-preinstalled-php-extensions/).

@tedivm
Copy link
Member

tedivm commented Mar 11, 2013

Yup, I actually opened the tickets with Travis about it-

travis-ci/travis-ci#970
travis-ci/travis-ci#971

I've also created a new branch to resolve the testing issues-

https://github.com/tedivm/Stash/tree/travis_test

Once it's resolved I'll merge the travis_test branch back into mainline, and then I'll ask you to just update your PR with those tests so we can see that things are running smoothly.

@lolautruche
Copy link
Contributor Author

Ok, @tedivm . Just let me know 😃

@andrerom
Copy link
Contributor

@lolautruche Travis should be fixed now, so you can do a git ci --amend & and a force push to get it to test this.

@lolautruche
Copy link
Contributor Author

@tedivm It seems that your tickets with Travis and Memcached are fixed now, and your travis_test branch looks it passes tests. Could you then merge your travis_test branch into master so I can rebase ?
Thanks !

@tedivm
Copy link
Member

tedivm commented Mar 18, 2013

The travis_test branch isn't actually passing them all- it's skipping the APC tests. I'm trying to get that resolved (I think I know why it's occurring) and merge that back into mainline. I"ll update here once that's handled.

@lolautruche
Copy link
Contributor Author

@tedivm Sorry to insist... Actually we really need this in for the next version of eZ Publish and we'll soon enter in a deep code freeze mode. This PR is not related to APC or Memcached since it's only about the FileSystem driver.
If you really want to wait your issue to be fixed, OK but I'll have to point eZ Publish on my fork which I don't really want :-/.

Thanks.

tedivm added a commit that referenced this pull request Mar 30, 2013
FileSystem driver now tries to create cache folder before throwing InvalidArgumentException
@tedivm tedivm merged commit 1606a2b into tedious:master Mar 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create FileSystem cache directory during compilation
4 participants