Page cache sweeper not working #1615

Closed
wants to merge 5 commits into
from

3 participants

@f3ng3r

I experienced that page_cache_directory = Pathname.new(page_cache_directory.to_s) returned an empty Pathname instance, probably because the page_cache_directory variable had the same name as the Rails setting.

Because of this it was unable to delete the right folder .../public/refinery/cache/pages.

Changing page_cache_directory to page_cache_directory_path fixed that for me.

@travisbot

This pull request passes (merged d53c59a into ef9f440).

@f3ng3r

There still seems to be problems with the cache sweeper, I will add more code to this pull request later (hopefully today). The current problem is that it cannot delete an unempty directory with the rmdir command.

Errno::ENOTEMPTY (Directory not empty - .../public/refinery/cache/pages)

@travisbot

This pull request passes (merged 9caa870 into ef9f440).

@parndt parndt and 1 other commented on an outdated diff Apr 23, 2012
pages/app/controllers/refinery/page_sweeper.rb
# Delete the full Cache
- if (cache_root = page_cache_directory.join('refinery', 'cache', 'pages')).directory?
- cache_root.rmdir
+ if (cache_root = page_cache_directory_path.join('refinery', 'cache', 'pages')).directory?
+ FileUtils.rm_rf cache_root
@parndt
Refinery member
parndt added a line comment Apr 23, 2012

Can this be

cache_root.rm_r
@f3ng3r
f3ng3r added a line comment Apr 23, 2012

No i just tested this, and rm_r is not a valid method for an instance of Pathname.

@parndt
Refinery member
parndt added a line comment Apr 23, 2012

Sorry I wrote that on my phone.. I meant rmtree

@f3ng3r
f3ng3r added a line comment Apr 23, 2012

Yes that seem to work. I will make a new commit with the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@f3ng3r

I added one more commit because the page cache html file for the root url is called pages.html and is placed inside .../public/refinery/cache/ alongside the pages/ directory, and it is therefore not being deleted when the pages/ directory is removed.

@travisbot

This pull request fails (merged e2e9fc0 into ef9f440).

@f3ng3r f3ng3r Forgot that i had removed `'pages'` from `cache_root = page_cache_dir…
…ectory_path.join('refinery', 'cache')`. Added it again.
d3ccf5c
@travisbot

This pull request passes (merged d3ccf5c into ef9f440).

@parndt parndt and 1 other commented on an outdated diff Apr 23, 2012
pages/app/controllers/refinery/page_sweeper.rb
# Delete the full Cache
- if (cache_root = page_cache_directory.join('refinery', 'cache', 'pages')).directory?
- cache_root.rmdir
+ if (cache_root = page_cache_directory_path.join('refinery', 'cache', 'pages')).directory?
+ FileUtils.rm_rf cache_root
+ end
+
+ # Delete the pages index file (/refinery/cache/pages.html)
+ if (cache_index = page_cache_directory_path.join('refinery', 'cache', 'pages.html')).file?
+ FileUtils.rm_f cache_index
@parndt
Refinery member
parndt added a line comment Apr 23, 2012
@f3ng3r
f3ng3r added a line comment Apr 23, 2012

Yes i changed that when I changed to .rmtree and it is in the commit i just made :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@f3ng3r f3ng3r use `cache_root.rmtree` instead of `FileUtils.rm_r cache_root`, use `…
…cache_index.delete` instead of `FileUtils.rm cache_index` and delete the gzipped version of the cache root_path file.
3e91796
@f3ng3r

Made fixes proposed by parndt and added that it removes the gzipped version of the root_path cache file pages.html.gz.

@travisbot

This pull request passes (merged 3e91796 into ef9f440).

@parndt
Refinery member

Merged into 2-0-stable as 6cef03a and master as b00695c thanks!

@parndt parndt closed this Apr 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment