3.0 - Convert configuration into a 'static' file #1575

markstory · 2013-08-30T02:39:34Z

This is a prototype/working demonstration of converting the various configuration files in CakePHP into a single 'static' file. I use quotes around static as the file is actually PHP. I originally tried to use ini files, but it negatively affected performance in a fairly significant way. An ini file was ~8% slower on my laptop, which didn't seem worth i. Additionally, ini files have more risk in lazy deployment scenarios as there is a risk of the app.ini being exposed accidentally. Interestingly, switching to the single config file boosted performance ~1%, which I assume came from the reduced number of calls to Configure.

I wanted to get a discussion started around this before doing much more work on it.

Is a single static config file a reasonable thing?
Do the new methods make sense and seem useful?

Once this issue is settled my plan is to finalize the work around composer and get the code to a point where the cakephp-app repo could be split off and be used for further development. See this wiki page for more details on those plans.

Phally · 2013-08-30T03:16:09Z

The speed increase can have other reasons too. INI files aren't stored in opcode cache because they aren't executable for example so they are parsed on every request. Also having a single config file and no opcode cache (or opcode cache with stat on) is still a win because there will be less file hits.

So I like the idea of having a single config file. One thing that I've seen in the wild is that people tend to use DATABASE_CONFIG::__construct() for dynamic selection of their data source. That won't be possible anymore so that is going to affect the migration to 3.0.

markstory · 2013-08-30T03:25:29Z

With a PHP code based config file it will still be possible. The config file and bootstrap.php are still code in App. If developers want to get fancy and use additional logic to define their connections they can by removing the ConnectionManager::config() call in Config/bootstrap.php and inserting the logic they require there.

Phally · 2013-08-30T11:34:48Z

Yeah I didn't mean it wasn't possible at all, just saying it will be different.

markstory · 2013-08-30T12:06:33Z

For sure. The lack of a a class containing the configuration is biggest change I think. I was planning on updating the docs to include an example of conditionally defined data sources.

It might also be a good idea to document how to split up the config file for deployment situations where not all of the config needs to be different.

markstory · 2013-08-31T01:41:29Z

I decided to remove Config/app.php and instead have Config/app.php.default I also tweaked the default config settings to suggest better practices around database access. I moved the config file because in the future I'd like to avoid causing merge conflicts with downstream forks if/when we modify the default configuration.

Phally · 2013-08-31T03:36:47Z

Yeah, I never like that class so I am not complaining. Having only a default and not a Config/app.php or Config/core.php is something I like as well. I always end up renaming them too, because they are different between environments.

This is a proof of concept that application configuration can be put into static files quite easily.

Using a .php extension prevents webservers from accidentally serving the configuration file as plain text. This is important for people who do sloppy deployments where the contents of App/ are accessible from the webserver.

Once configuration data has been injected into the various classes it is no longer necessary in Configure. Keeping it in configure only consumes more memory and give the impression that changing data in configure will update the other classes.

This is a simpler/more efficient method for bootstrapping configuration.

- Use consume() it is faster than read() + delete(). - Do not merge config as Configure should be empty and merging is a waste of time.

There is a significant performance drawback (approx. -8%) to using ini files. Using PHP based files actually increases performance (approx. 1.1%) as there are fewer calls to Configure overall. Using PHP files has the advantage of being harder to screw up in various deployment scenarios.

Including a half working config file is probably not a good idea. Move the config file out of the way and force developers to create one.

Save on a few lines of code with inline constructors.

lorenzo · 2013-08-31T18:49:18Z

App/Config/bootstrap.php

 */
-require __DIR__ . '/cache.php';
+if (!class_exists('App\Controller\AppController')) {


We need another way of doing this, looks like obscure magic.

This will go away once we finish transitioning to requiring composer.

jrbasso · 2013-08-31T18:50:10Z

👍

lorenzo · 2013-08-31T22:30:38Z

Sweet 👍

3.0 - Convert configuration into a 'static' file

CauanCabral · 2013-09-02T01:32:08Z

App/Config/app.php.default

+	],
+
+/**
+ * Security ane encryption configuration


Would be "Security and encryption..."?

Yep, I suck at keyboards.

Fixed in ffde282

ravage84 · 2013-09-02T19:59:01Z

It might also be a good idea to document how to split up the config file for deployment situations where not all of the config needs to be different.

I think it would make even more sense to provide a built-in solution for that, because almost everybody needs to take care of deploying to different environments where not all config changes (e.g. local dev, central dev server, testing server, production server).

How about adding another file? Let's say app.local.php.
This one would be empty (=no config values) by default.
In bootstrap.php we would load both files but the config values in app.local.php would overwrite the ones in the renamed app.php.
Alternativly we could not add app.local.php by default but load it only when it exists (probablyspeedier).

This way one could create a app.local.php for each (differing) environment he has.
I guess most of the time only the debug mode and the database settings will be in app.local.php.

A bit similiar to the way DokuWiki's configuration works.

markstory · 2013-09-02T20:08:51Z

@ravage84 Right now there is only a app.php.default file included. Would it be useful to include a commented out line for the app.local.php file as well? That would give people a simple example of how to both split up config files and provide environment specific config overrides.

ravage84 · 2013-09-02T20:15:36Z

Yes I think that will do. If people need more sophistication they could work it out themselve. If they don't need or want 'local' configs, they could ignore that line.

markstory added 14 commits August 31, 2013 11:19

Start moving app config into an ini file.

769888d

This is a proof of concept that application configuration can be put into static files quite easily.

Add sample email and email transport config.

7431544

Remove now unused config files.

e2d1f14

Move app.ini to app.ini.php

aed6247

Using a .php extension prevents webservers from accidentally serving the configuration file as plain text. This is important for people who do sloppy deployments where the contents of App/ are accessible from the webserver.

Remove now useless configuration data.

b76eb3a

Once configuration data has been injected into the various classes it is no longer necessary in Configure. Keeping it in configure only consumes more memory and give the impression that changing data in configure will update the other classes.

Add Configure::consume()

e96e902

This is a simpler/more efficient method for bootstrapping configuration.

Use more efficient methods during bootstrapping.

673896d

- Use consume() it is faster than read() + delete(). - Do not merge config as Configure should be empty and merging is a waste of time.

Add missing use statement.

5d0b19f

Move app.php to app.php.default

476a38b

Including a half working config file is probably not a good idea. Move the config file out of the way and force developers to create one.

Update bootstrapping in the case where Config/app.php does not exist.

ab07859

Update README with setup instructions.

c83cd98

Update to latest 3.0 changes.

5819e20

Use inline constructors.

3cc489e

Save on a few lines of code with inline constructors.

lorenzo reviewed Aug 31, 2013
View reviewed changes

Change comment style on default config file.

8933e24

lorenzo added a commit that referenced this pull request Aug 31, 2013

Merge pull request #1575 from markstory/3.0-config

87b2f4b

3.0 - Convert configuration into a 'static' file

lorenzo merged commit 87b2f4b into cakephp:3.0 Aug 31, 2013

CauanCabral reviewed Sep 2, 2013
View reviewed changes

markstory mentioned this pull request Sep 3, 2013

3.0 - local config #1602

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3.0 - Convert configuration into a 'static' file #1575

3.0 - Convert configuration into a 'static' file #1575

markstory commented Aug 30, 2013

Phally commented Aug 30, 2013

markstory commented Aug 30, 2013

Phally commented Aug 30, 2013

markstory commented Aug 30, 2013

markstory commented Aug 31, 2013

Phally commented Aug 31, 2013

lorenzo Aug 31, 2013

markstory Aug 31, 2013

jrbasso commented Aug 31, 2013

lorenzo commented Aug 31, 2013

CauanCabral Sep 2, 2013

markstory Sep 2, 2013

markstory Sep 2, 2013

ravage84 commented Sep 2, 2013

markstory commented Sep 2, 2013

ravage84 commented Sep 2, 2013

3.0 - Convert configuration into a 'static' file #1575

3.0 - Convert configuration into a 'static' file #1575

Conversation

markstory commented Aug 30, 2013

Phally commented Aug 30, 2013

markstory commented Aug 30, 2013

Phally commented Aug 30, 2013

markstory commented Aug 30, 2013

markstory commented Aug 31, 2013

Phally commented Aug 31, 2013

lorenzo Aug 31, 2013

Choose a reason for hiding this comment

markstory Aug 31, 2013

Choose a reason for hiding this comment

jrbasso commented Aug 31, 2013

lorenzo commented Aug 31, 2013

CauanCabral Sep 2, 2013

Choose a reason for hiding this comment

markstory Sep 2, 2013

Choose a reason for hiding this comment

markstory Sep 2, 2013

Choose a reason for hiding this comment

ravage84 commented Sep 2, 2013

markstory commented Sep 2, 2013

ravage84 commented Sep 2, 2013