Closed (duplicate)
Project:
Drupal core
Version:
8.0.x-dev
Component:
configuration system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
24 Sep 2012 at 07:48 UTC
Updated:
29 Jul 2014 at 21:12 UTC
Jump to comment: Most recent file
Comments
Comment #1
kim.pepperInitial patch. I created a new config file called system.reverse-proxy.yml in the system module.
Kim
Comment #2
swentel commentedLooks good overall to me, one tiny little nitpick though:
Extra space after 'is' should be removed.
Comment #3
kim.pepperRemoved trailing space and renamed config variable for consistency.
K
Comment #5
kim.pepper#3: 1793102-3-convert-reverse-proxy-settings-to-cmi.patch queued for re-testing.
Comment #7
kim.pepper#3: 1793102-3-convert-reverse-proxy-settings-to-cmi.patch queued for re-testing.
Comment #9
kim.pepperRe-rolled against latest 8.x
Comment #11
kim.pepperI'm getting an exception 'PDOException' with message 'SQLSTATE[08004] [1040] Too many connections' from testbot.
Comment #12
kim.pepperComment #13
kim.pepper@tim.plunkett pointed me to a possible known issue that could be the cause #843114: DatabaseConnection::__construct() and DatabaseConnection_mysql::__construct() leaks $this (Too many connections)
Comment #14
kim.pepper#9: 1793102-9-convert-reverse-proxy-settings-to-cmi.patch queued for re-testing.
Comment #16
kim.pepperRe-rolled against latest 8.x-dev. Still no idea why these tests passed but are now failing...
Comment #18
kim.pepperStill getting "Too Many Connections" errors.
I have run these failing tests locally and they all pass.
Comment #19
kim.pepperUnassigning as I can't think of anything else that could cause this. :-(
K
Comment #20
kim.pepperRebased off latest 8.x
Still don't know why this is failing. Looking for someone to provide some help.
Comment #22
kim.pepperupdate hook bump
Comment #24
kim.pepper"Too many connections" GAAAAAAAH!
Comment #25
kim.pepperRe-roll
Comment #26
kim.pepper@heyrocker says we are using 0/1 for booleans in yaml files.
Comment #27
kim.pepperFixed whitespace
Comment #28
kim.pepperFixed quoting in yaml as per http://drupal.org/node/1696902#comment-6275656
Comment #29
larowlanLooks good
Comment #30
catchI don't think this should necessarily be converted to config at all for several reasons:
- whether there's a reverse proxy or not, and what those are, may differ between staging and production. We don't have a concept of environment-specific configuration in the config API yet, or a recommended way to handle this.
- these settings were always meant to be hard-coded in settings.php, because ip_address() is called in drupal_anonymous_user() and watchdog() - so this is adding a config() dependency very, very low down in the system.
- It's also going to add database requests where we previously had none.
Not moving this to CNW, but it needs more discussion. The alternative is to move these globals out of $conf (similar to global $databases).
Also if we do actually convert this to CMI, then it shouldn't be listed in settings.php since that's not going to be the recommended way to set it up any more.
Comment #31
kim.pepper@catch, thanks for your feedback.
My understanding is that Symfony has the concept of dev and prod, but not sure if it handles multiple environments. This is handled in http://drupal.org/project/env in a similar way to Ruby on Rails, where config for each environment is checked into source control.
Damn. Is that the case for all config? My understanding of the config system is limited.
Kim
Comment #32
gddEach request to config is a file system hit when the cache is cold, and a database query when its warm. There are issues open to begin addressing this with static caching and other possibilities. But its probably going to wait until after feature freeze before it goes anywhere.
Managing contextual config differences between dev and prod will most likely be solved by contrib using the new contextual override system (same thing multilingual is using.)
I do agree that this is almost always going to be different between dev and prod, and thus probably isn't best placed in config. I think catch's suggestion of making it a hardcoded variable ala $databases is not a bad idea. Another possibility would be the state system, but that will add the same overhead that config does in terms of db queries (I think.)
Comment #33
kim.pepperThis is now fixed in #1833516: Add a new top-level global for settings.php - move things out of $conf
Comment #33.0
kim.pepperFixed copy/paste error