-
Notifications
You must be signed in to change notification settings - Fork 397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update configuration management documentation #1154
Conversation
@geerlingguy I'm going to wait for your feedback before merging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some notes in the PR, let me know what you think! (Otherwise looks pretty good!)
readme/configuration-management.md
Outdated
Note that as of version 8.3.3, Features can manage user roles and permissions, but not in an independent fashion. Permissions can only be exported for an entire role at once, unlike in D7 where you could export roles and their associated permissions separately. For this reason, Features excludes roles and permissions by default. If you wish to export them, change the "alters" setting on your Features bundle. ([reference](https://www.drupal.org/node/2383439)) | ||
- Database updates: the equivalent of running `drush updb` or hitting `update.php`, this applies any pending database updates. | ||
- Config import: runs the core configuration-import command to import any configuration stored in the root `config` directory. This is either a full or partial import, depending on how BLT is configured. | ||
- Features import: runs features-import-all, which imports any configuration stored in a feature module's `config/install` directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't seem clear from the above language that any of these things are configurable (e.g. if I am not using features, it looks like this says BLT will still run a features-import-all
. Can you mention the fact that features import will run iff Features is being used?
readme/configuration-management.md
Outdated
@@ -42,6 +52,45 @@ The solution is to make sure that the referenced content exists before the featu | |||
* Use the default_content module to export the referenced content as JSON files, and store these files with your feature or in a dependency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two lines should be updated to be like "store these files with a custom feature or module"—basically genericize the features language.
readme/configuration-management.md
Outdated
|
||
The best way to handle this is to always follow these steps when updating contributed and core modules: | ||
|
||
1. Start from a clean local setup or refresh. If you are using Features, ensure that there are no overridden configuration. The `cm.features.no-overrides` flag in [project.yml](https://github.com/acquia/blt/blob/8.x/template/blt/project.yml#L62) can assist with this by halting builds with overridden features. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe "Start from a clean local:setup
or local:refresh
to be more specific.
readme/configuration-management.md
Outdated
The best way to handle this is to always follow these steps when updating contributed and core modules: | ||
|
||
1. Start from a clean local setup or refresh. If you are using Features, ensure that there are no overridden configuration. The `cm.features.no-overrides` flag in [project.yml](https://github.com/acquia/blt/blob/8.x/template/blt/project.yml#L62) can assist with this by halting builds with overridden features. | ||
2. Use `composer update` to download the new module version(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to recommend plain composer update
(which can have a lot of unintended consequences), or more specific composer update drupal/modulename --with-dependencies
(which seems to be more foolproof for someone who isn't meticulous about monitoring what happens to composer.lock
).
readme/configuration-management.md
Outdated
1. Start from a clean local setup or refresh. If you are using Features, ensure that there are no overridden configuration. The `cm.features.no-overrides` flag in [project.yml](https://github.com/acquia/blt/blob/8.x/template/blt/project.yml#L62) can assist with this by halting builds with overridden features. | ||
2. Use `composer update` to download the new module version(s). | ||
3. Run `drush updb` to apply any pending updates locally. | ||
4. If any updates were applied, check if they modified any stored configuration. If using Features, simply check for overridden features. If using core CM, re-export all configuration and check for any changes on disk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"If any updates were applied, check if they modified any stored configuration. If using Features, check for overridden features. If using core CM, export all configuration and check for any changes on disk using git status
."
readme/configuration-management.md
Outdated
2. Use `composer update` to download the new module version(s). | ||
3. Run `drush updb` to apply any pending updates locally. | ||
4. If any updates were applied, check if they modified any stored configuration. If using Features, simply check for overridden features. If using core CM, re-export all configuration and check for any changes on disk. | ||
5. Re-export and commit any changes you found in the previous step, along with the updated `composer.json` and `composer.lock`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Export any changed Features (if using Features) and commit..."
readme/configuration-management.md
Outdated
4. If any updates were applied, check if they modified any stored configuration. If using Features, simply check for overridden features. If using core CM, re-export all configuration and check for any changes on disk. | ||
5. Re-export and commit any changes you found in the previous step, along with the updated `composer.json` and `composer.lock`. | ||
|
||
We need to find a better way of preventing this than manually monitoring module updates. Find more information in [these](https://www.drupal.org/node/2745685) [issues](https://github.com/acquia/blt/issues/842). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might just be me, but I'd rather have it formatted like:
Find more information in these issues:
- #XYZ: Linked issue 1 title
- #XYZ: Linked issue 2 title
readme/configuration-management.md
Outdated
|
||
Note that this workflow currently has two major limitations. The first is that individual configurations can't be entirely excluded from configuration management. For instance, if you want administrators to be able to set the site name (as above) or create new contact forms / webforms in production, this would be difficult given the current state of the module. However, this should theoretically be possible. | ||
|
||
TODO: Update once we figure out the graylist, and with additional information about how to integrate with BLT. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For merge, we can put "TODO: Update this documentation once Config Split's greylist functionality has been better documented and tested."?
readme/configuration-management.md
Outdated
@@ -143,4 +178,6 @@ If you need to override the default configuration provided by another project (o | |||
|
|||
### Other gotchas | |||
|
|||
Be aware that reverting all features and config on every deploy creates a risk of discarding server-side changes. This risk should be controlled by carefully managing permissions, and must be balanced against the greater risk of allowing for divergent configuration between your DB and VCS. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add one more gotcha that Configuration Management in Drupal 8 is still being improved early in the Drupal 8 lifecycle, and you should continue to monitor Drupal Core's issue queue and Drupal Planet blog posts for refinements to the CM workflows explained here.
@danepowell @geerlingguy @grasmash This is really great, but it might be nice to leave the Features docs in-place and add a note that Config Split is preferred. The Features workflow information is still very valuable for anyone that has not moved over the the Config Split yet. |
@greylabel I don't think we actually discarded any of the Features docs here, did we? We just rolled them into this more generic document as one of two options (config split vs features). |
@danepowell My bad. I should read more closely. I had been following the commits on this issue, but saw the features-workflow.md was removed and the final merge and assumed wrong that all Features content went with it. |
No worries, I know there was a ton of red in that diff and it might not be easy to follow the changes. |
The PS config management working group has agreed to try to standardize on a config_split based workflow. This PR makes the existing documentation a little more generic (less specific to features), and focuses on Config Split.
@geerlingguy I'd love your opinion on this, especially the sections on config_split that I'm less familiar with. If you think it's a good start, maybe we merge as-is and then you can take a pass at cleaning it up?
Also, I'm wondering if instead of making it so generic, we should have a "quick start" section at the beginning, i.e. "here's how to use BLT with config_split in 5 easy steps".