Remove the variable_get inline in CacheFactory::getBackends [#1772682]

Comment	File	Size	Author
#4	1772682_4.patch	932 bytes	chx
#3	drupal8.cache-classes.3.patch	2.98 KB	sun

Comment #1

chx commented 3 September 2012 at 22:23

Title:

Remove the variable_get inline in ConfigFactory::getBackends

» Remove the variable_get inline in CacheFactory::getBackends

Log in or register to post comments

Comment #2

catch

he/him

English

commented 4 September 2012 at 12:19

I'd like to identify everything that's currently in $conf but only designed to be set in settings.php and move it out of there - even if it's only to some other global namespace. That's the third part of removing variable_get() after CMI and the state system.

We could do global $settings, global $drupal_settings. Or we could go for one-off globals like the $databases array - so add global $cache_backends or similar? I'd be fine with any of those options. We could also try to move bootstrap configuration out of a raw global but it'd be completely fine to do this in two steps.

Log in or register to post comments

Comment #3

sun

German

Karlsruhe

commented 5 September 2012 at 00:27

Status:

Active

» Needs review

Status	File	Size
new	drupal8.cache-classes.3.patch	2.98 KB

Makes sense to me.

At the very least, it's out of $conf with this, for which the current definition is incompatible. If it was to stay in $conf, then it would have to be re-keyed onto $conf['system.performance']['cache']['classes'] (or similar). But for now, a separate $cache_classes global seems to make more sense.

In a later step, we can explore whether the bin options could be part of a cache.factory service definition on a bootstrap container object that is read from a /sites/[site]/services.yml config file. (see #1703266-5: Use a precompiled container for the minimal bootstrap container required outside of page requests)

Log in or register to post comments

Comment #4

chx commented 5 September 2012 at 02:06

Status	File	Size
new	1772682_4.patch	932 bytes

I think both #2 and #3 are totally off topic. Looking at $conf['system.performance']['cache']['classes'] I imagine sun thinks $conf becomes part of CMI? Was that the reason you inlined the variable_get into CacheFactory? I guess it was because the comment in getBackends also makes it look like that.

Well, I am just about to roll the state API patch and variable becomes part of that instead of CMI. But that is completely off topic too. It is not the scope of this issue to solve the variable_gets used for bootstrap. They should be solved together and not piecemeal. So, this patch is as simple as attached (and this is what I did in the parent issue as well just didn't want to distract that one with this debate).

Log in or register to post comments

Comment #5

fubhy commented 2 October 2012 at 09:32

What we actually need is a full list of all cache bins that are currently available. We can only really solve that by registering the bins in some way. Cache tag invalidation, garbagecollection, etc. has to run per-bin and there are more and more issues (#1167144: Make cache backends responsible for their own storage) coming up for which it would be useful to have a global list of existing bins.

I would suggest to go with the approach from #1167144: Make cache backends responsible for their own storage but for all bins instead of just the database backend-based bins.