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

Add assets.cache_package_paths and unit test. #133

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add assets.cache_package_paths and unit test. #133

wants to merge 1 commit into from

Conversation

niallsmart
Copy link
Contributor

The current version of AssetPack always caches the list of files in a package between requests. This is not ideal in a development environment, when files are being added or reorganized fairly frequently.

This PR adds support for a new configuration option cache_package_paths which can be used to control the cache behavior. It disables caching of package contents in the development environment by default.

@niallsmart
Copy link
Contributor Author

FYI – I also considered the name cache_package_contents but that implied it was actually caching the compiled contents. Another good option might be cache_package_definitions or cache_file_list.

@j15e
Copy link
Collaborator

j15e commented Aug 18, 2013

Hello, Niall, thanks for the contribution but I think we are getting lost with caching optoons.

This is not your fault but I think this is getting messy, we would now have 3 options related to the caching of various parts of the asset manager with existing reload_files_cache, dynamic_asset_cache & this new one.

I think a rewrite of the caching option should be done to get only 1 caching option, all on or all off, not many small specific caching options that are getting complex to understand. Or, if we really want to keep specific options, they should be a bit more standardized & regrouped.

So for this particular reason I won't merge in master for now as-is although it be very tempting.

I won't work on a cleanup myself right now as I have less time available for the maintenance of this project : this is open for contributions.

@niallsmart
Copy link
Contributor Author

@j15e yes I agree, it is getting a little confusing. I'm not even sure of the interactions between AssetPack's cache and Sinatra's caching. I will read through the code a little more and make a proposal that should hopefully result in simpler code/API. I do think it's important though to enable the developer to disable caching of the package file lists though, it's a bit of a pain to have to restart the server every time a new file is added or a file is renamed.

@dentarg
Copy link
Contributor

dentarg commented Oct 14, 2013

@niallsmart Not a real solution, but I use https://github.com/alexch/rerun to restart the server when things happen in the filesystem. It works really well.

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.

3 participants