Both file and db configuration storage are said to be default in settings.php

I'll try to make a patch today

This is really a doc issue - just changing comments or commented-out lines.

added more explanation

+ # $GLOBALS['conf']['container_yamls'][] = $conf_path . '/services.yml';

Hm. Not sure whether we want to document that, given #2241633: Simplify site-specific service overrides

As a follow-up to that issue, I'd actually recommend to remove the $GLOBALS['conf']['container_yamls'] variable entirely, and instead, make the kernel additionally check for a environment-specific services.$env.yml file.

Aside from that, this looks relatively good to me. I only wondered whether there's a possibility to shorten the text in any way?

Created #2244807: Replace $GLOBALS['conf']['container_yamls'] with optional $conf_path/services.$env.yml file

This is a huge improvement! Two minor nitpicks:

+++ b/sites/default/default.settings.php
@@ -216,12 +216,21 @@
+ * By default, Drupal's active configuration is stored in the database.
+ * However, configuration import occurs using files. By default the staging

These should probably both start, "By default," (with a comma).

+++ b/sites/default/default.settings.php
@@ -216,12 +216,21 @@
+ * configuration. See below the active configuration settings documentation.

I'd change this to "See the active configuration settings documentation below."

Minor wording tweaks as suggested.

I think this still needs some work. Starting with the first paragraph:

+ * By default, Drupal's active configuration is stored in the database, while
+ * configuration import occurs using files. By default, the staging  directory
+ * used for configuration import is found in a randomly named directory under
+ * the default public files path. On install the directory is created in the
+ * default files directory. For enhanced security, you may set the path to a
+ * location outside your docroot.

a) The word "default" and phrase "by default" occur way too many times. It's kind of hard to read.
b) There's an extra space near the end of the 2nd line here.
c) This isn't actually accurate. The config directly is the random-named directly that is created, and under that, there's "active" and "staging".
d) "docroot" is also not a word. Say "document root" please.
e) This paragraph is a mix of talking about config storage and config import and config directories. It's very confusing. Start at the beginning (what this setting is for: the config directories), and then explain what the config directories are used for, maybe?

Second paragraph:

+ *
+ * On install, Drupal will also set up an active configuration directory next
+ * to the staging directory. The active directory is expected to be empty unless
+ * you change the configuration system to use file storage for the active
+ * configuration. See the active configuration settings documentation below.

f) What does "next to" mean?

Third paragraph:

+ *
+ * If you use files for the active configuration in production you may enhance
+ * your security by setting the path to the active configuration to a location
+ * outside your docroot.

g) See (d).

Last section of patch:

+ * module or profile, or by adding to the container YAML files for DrupalKernel.
+ *
  *
  * The 'bootstrap_config_storage' setting needs to be a callable that returns

h) Extra blank line has been added.

Very end of patch:

  * The 'bootstrap_config_storage' setting needs to be a callable that returns
- * core.services.yml.
+ * an instance implementing \Drupal\Core\Config\StorageInterface.
  */
  # $settings['bootstrap_config_storage'] = array('Drupal\Core\Config\BootstrapConfigStorageFactory', 'getFileStorage');
+ # $GLOBALS['conf']['container_yamls'][] = $conf_path . '/services.yml';

i) That GLOBALS line is not explained, is it?

And then...
j) The active and staging config directories are given README files when you install. These I think also need an update in this patch.

Attempting a-h. Thoughts?

j

Much better thanks!

Still some things to fix:

a) install.inc
... By default, this directory will be empty. To move this configuration between environments,...
So by default it is empty, and then you can move the configuration??? Maybe say "If you are using files to store configuration, and you want to move it between environments..." or something like that?

b) default.settings.php

  * By default, the active configuration is stored in the database in the
  * {config} table. To install Drupal with a different active configuration
- * storage, you need to override the setting here, in addition to overriding
- * the config.storage.active service definition in a module or profile.
+ * storage, you need to override the 'bootstrap_config_storage' setting here, in
+ * addition to overriding the config.storage.active service definition in a
+ * module or profile, or by adding to the container YAML files for DrupalKernel.
  *
  * The 'bootstrap_config_storage' setting needs to be a callable that returns
- * core.services.yml.
+ * an instance implementing \Drupal\Core\Config\StorageInterface.
  */
  # $settings['bootstrap_config_storage'] = array('Drupal\Core\Config\BootstrapConfigStorageFactory', 'getFileStorage');
+ # $GLOBALS['conf']['container_yamls'][] = $conf_path . '/services.yml';

I'm still confused here. If someone uncommented these two lines, what else would they need to do in order to get this to work, so that they are using files as their active config storage? And shouldn't it be pretty easy to do that, since I think the services/classes to do this already exist in Core, don't they??

So can we instead give explicit instructions here, like "Uncomment these two lines, change X to Y, and they in a module, do Z"? I just don't understand the instructions that are in this patch (or the ones that were in the file before the patch, for that matter). And I have no idea whether or not the $GLOBALS line is correct or not either.

This is just the README change.

I'd prefer to wait until #2241633: Simplify site-specific service overrides has landed, so we do not have to document how to futz with global variables here, because futzing with global variables is something we shouldn't document.

@sun Makes sense.

This issue is in, so we should be able to update the patch now: #2241633: Simplify site-specific service overrides

straight reroll of #17, I haven't addressed #18.

#23 looks good to me.

@sun, #18 still needs to be addressed, right? or did it go away?

It looks like the patch still adds a GLOBALS and according to sun/comment #18 this should go away, so marking Needs Work. Someone needs to read the other issue (or its change notice) and figure out what changed, or maybe sun can make a suggestion?

23: 2243439-23.patch queued for re-testing.

Getting ready for the Austin DrupalCon sprint, following http://www.hook42.com/blog/prepping-drupalcon-austin-sprints-sprint-lead...

Can we please leave this issue at Needs Work until the issues in #18/#26 are addressed, thanks!

How about this?

Better! Now I think we can bikeshed about the quality of the documentation. I have a few thoughts:

a) In install.inc:

To make this configuration active, see admin/config/development/configuration/sync on the target server.

I don't think we "see" URLs.... How about "To import this configuration, visit ..." ?

b) First chunk of default.settings.php:

+ * The $config_directories array specifies the location of file system
+ * directories used to store Drupal's configuration. By default, the active
+ * configuration is stored in the database and configurations to be imported are
+ * stored in a staging directory. On install, the staging directory is created
+ * inside a randomly-named directory in the public files path.
+ *
+ * On install, Drupal will also set up an active configuration directory inside
+ * the same randomly-named directory in the public files path. The active
+ * directory is expected to be empty unless you change the configuration system
+ * to use file storage for the active configuration. See the active
+ * configuration settings documentation below.
+ *
+ * If you use files for the active configuration in production you may enhance
+ * your security by setting the path to the active configuration to a location
+ * outside your document root.

There is a ton of redundancy in the first two paragraphs, and I don't think "configuration" should be pluralized... How about:

The $config_directories array specifies the location of file system directories used for configuration data. On install, "active" and "staging" directories are created for configuration. The staging directory is used for configuration imports; the active directory is not used by default, since the default storage for active configuration is the database rather than the file system (this can be changed; see "Active configuration settings" below).

The default location for the active and staging directories is inside a randomly-named directory in the public files path; this setting allows you to override these locations. If you use files for the active configuration, you can enhance security by putting the active configuration outside your document root.

c) Last chunk in default.settings.php (I omitted the - lines for clarity):

  * By default, the active configuration is stored in the database in the
  * {config} table. To install Drupal with a different active configuration
+ * storage, you need to override the 'bootstrap_config_storage' setting here, in
+ * addition to overriding the config.storage.active service definition such as
+ * in a services.yml file in the same directory as settings.php which is used to
+ * register site-specific service definitions.
  *
  * The 'bootstrap_config_storage' setting needs to be a callable that returns
+ * an instance implementing \Drupal\Core\Config\StorageInterface.

I found this pretty hard to scan... How about:

By default, the active configuration is stored in the database in the {config} table. To use a different storage mechanism for the active configuration, do the following prior to installing:
- Override the 'bootstrap_config_storage' setting here. It must be set to a callable that returns an object that implements \Drupal\Core\Config\StorageInterface.
- Override the service definition 'config.storage.active'. Put this override in a services.yml file in the same directory as settings.php (definitions in this file will override service definition defaults).

Thoughts?

Can we leave the edits for a novice to attempt on the June 6 Austin sprint please?

Maybe, but I am usually against "reserving" issues for sprints... I understand wanting to have issues that are appropriate for novices to work on at a sprint, but there is no guarantee that if we ask someone not to work on an issue today, that it will get fixed tomorrow at the sprint.

I'd prefer not to discourage anyone from fixing any issue at any time. There are also people at DrupalCon all week getting mentored, not necessarily at a formal "sprint", and doing great work on issues, so reserving this for a particular sprint session doesn't seem like the right thing for those people either?

I'm a novice, I will check this out :)

How does this look to you all?

35: 2243439-35.patch queued for re-testing.

@mark.labrecque If the patch doesn't apply, try a simple git diff instead of git format-patch.

Thanks for the tip! I will do that

Also be sure to use unique patch file names that match the comment number.

Fingers crossed...

Looks generally good, but not sure why this is indented:

+ *      The $config_directories array

Ah, I missed that. Thanks for pointing that out.

Patch looks good to me.

Looking at this now as a second review.

Patch applies. Documentation looks good.

One caveat:

In applying the patch, I received a permissions issue on the second file change (default.settings.php). I had to roll back and manually change the rights on the default folder for the patch to apply. This won't be an issue for new installs once the changes are rolled into core.

Webchick mentioned this is a known issue and suggests:

sudo git checkout -- sites/default
OR
chmod ug+w sites/default

Looking better, thanks for the patch(es)!

A few notes:
a) install.inc

... If you are using files to store the active configuration, and you want to move it between environments, contents from this directory should be placed ...

I don't think "contents from this directory" makes sense... maybe "files from this directory"?

b) default.settings.php

  * For enhanced security, you may set this variable to a value using the
+ * contents of a file outside your document root that is never saved together
  * with any backups of your Drupal files and database.

Could we fix up the wording here? "never saved together with" ?!? "value using the contents of a file" ?!? What is this actually saying? And... this is actually out of scope for the issue here (it is in the "salt" section)... but if we are going to change this line anyway, could we fix it so it actually makes sense? My suggestion:

For enhanced security, you may set this variable to the contents of a file outside your document root; you should also ensure that this file is not stored with backups of your database.

c) default.settings.php

+ * {config} table. To use a different storage mechanism for the active
+ * configuration, do the following prior to installing:
+ *   - Override the 'bootstrap_config_storage' setting here. It must be set to a
+ *     callable that returns an object that implements
+ *     \Drupal\Core\Config\StorageInterface.
+ *   - Override the service definition 'config.storage.active'. Put this
+ *     override in a services.yml file in the same directory as settings.php
+ *     (definitions in this file will override service definition defaults).

List formatting: the - in list items should line up with the text above. So the entire list needs 2 spaces less indentation.

Other than that, looks good, thanks!

Please review revised patch with fixes in #53.

Looks good, thanks!

Thanks again everyone! Committed to 8.x.

What does:

Override the service definition 'config.storage.active'. Put this
 *   override in a services.yml file in the same directory as settings.php

actually mean to do?

what exactly should I put in the yml file?

config.storage.active: true

?

actually mean to do?

haven't tried, in theory yes.

what exactly should I put in the yml file?

We need to define active storage. Say, if we want to use fileStorage by default:

# sites/default/services.yml

services:
  config.storage.active:
    class: Drupal\Core\Config\FileStorage
    factory_class: Drupal\Core\Config\FileStorageFactory
    factory_method: getActive

YesCT: Do you want to reopen this issue, or file a new one? If you think the docs need to be clarified, we can do that. But I think if someone is going to do this, they'll hopefully realize they need to figure out how to override a service and look at the Services and Dependency Injection Container topic on api.drupal.org (linked from the api.d.o landing page) and figure out how to define a service.

Opened #2291587: Document how to toggle to file configuration storage in settings.php

Related Question:
http://drupal.stackexchange.com/questions/151604/is-there-any-sensative-...

Please post an answer if you know it. :)