Settings cache & clean up "ghostGlobals" #166
Conversation
| ghost.updateSettingsCache(result); | ||
| return result; | ||
| }); | ||
| } else { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
Connect has this Cache class that might be useful for us to use here but I'm not sure how we'd get to it. Doesn't seem like they export it on the connect module. |
|
Wouldn't it be simpler to load the settings on init as it's done, but then only add a hook to the API's |
|
Why would you not use the cache to return for requests to browse/read? This way settings has a pre-loaded cache layer that we can depend on to only make DB requests when it has to - in my mind that's how a cache should work. In all likelihood this is a very short-term solution, as I imagine we will implement pretty heavy caching across the whole DB layer, the only reason for doing settings primitively to start with is that the pre-loading is required, and this sort of fell out as a natural solution to the other problems which resulted from that. |
|
We're on the same page re. a general API cache. I just thought it would be good to keep the separation between application-level stuff and the API layer, for simplicity' sake. |
- ghost.js - globals/globalConfig has become settings / settingsCache to make it clearer - app.js - the ghostGlobals local cache is gone, and the use of res.locals has been cleaned up and simplified, although this needs to be properly split into frontend and admin locals (to be finished in TryGhost#124) - frontend/index.js - doesn't need to be passed globals and nav properties as res.locals does this for us
|
You're most probably right. This version fixes the issues for now though & I really want to get 0.1.1 out the door. So I've raised an issue (#173) for implementing a more permanent caching solution and we'll review the separation at that point too. |
- ghost.js - split the settings loading out of ghost.init, so that we have a function for loading / reloading settings - api.js - implemented a new requestHandler, the cachedSettingsRequestHandler which handles all aspects of local caching for settings when making requests - app.js - updated the settings api routes to use the new cached request handler
Settings cache & clean up "ghostGlobals"
Settings cache & clean up "ghostGlobals"
Please see #165
These two commits clean up the concept of globals, making it more explicit that these are settings from the database. The matching change for Casper is sitting in TryGhost/Casper#11
I have also implemented more explicit caching with reloading when a change occurs. This means the frontend will update with a new blog title when changed in the settings without restarting the server.
Would really appreciate a few eyes on this 😄