-
Notifications
You must be signed in to change notification settings - Fork 168
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
Check for existing persistent cache files on demand #26
Conversation
I've been using the changes in this pull request for several weeks and they works great! Thanks for doing this. Thought I'd share another benefit I got out of lazy-loading the cache files. When authoring tests that dynamically hit external APIs, a lot of extra cache files can be created that you don't want in the codebase. With the changes in this pull request, I was able to implement the following in my spec_helper.rb to delete any cache files not used when running the full test suite via config.after(:suite, type: :feature) do
if ENV['DELETE_UNUSED_CACHE'] && defined?(Billy)
puts "\n*** ALL UNUSED CACHE FILES WILL BE DELETED ***"
local_cache_path = Rails.root.join(Billy.config.cache_path)
used_cache_files = Billy.proxy.instance_variable_get(:@cache)
Dir.glob(local_cache_path+'*.yml') do |full_filename|
filename = full_filename.gsub(local_cache_path.to_s, '')
unless used_cache_files[filename.gsub('.yml','')]
puts " - DELETING #{filename}"
File.delete(local_cache_path+filename)
end
end
end
end |
@ronwsmith I'm glad you found the pull request useful. I like your deletion logic and I will try that out. |
Check for existing persistent cache files on demand
Merged, thanks! |
Thanks for merging this in! |
When I was using puffing_billy I noticed that for the first request it did not check for a persisted cache file. Later, I realized that I was not calling Billy.proxy.restore_cache in my config and also did not notice the Billy.cache.load_dir method.
However, I had already written additional logic to check for a persisted cache entry on demand. And I think this may be an improvement for the existing approach for a few reasons: