Skip to content
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

Add a --config parameter to import config after install. #1875

Closed
wants to merge 4 commits into from
Closed

Add a --config parameter to import config after install. #1875

wants to merge 4 commits into from

Conversation

simesy
Copy link

@simesy simesy commented Dec 18, 2015

Passing a --config parameter (eg. drush site-install --config=path/to/config) triggers the equivalent of a drush config-import --source=path/to/config after the installation is complete.

This is all-or-nothing, any UUIDs generated during the site installation will be overwritten, any config created during the installation that is not represented in the --config directory will be deleted.

A use case for feature is CI/testing where the source directory contains a known set of configuration to be tested.

For history of this feature see #1635

if ($config = drush_get_option('config')) {
// Set the destination site UUID to match the source UUID, to bypass a core fail-safe.
$source_storage = new FileStorage($config);
drush_op('drush_config_set', 'system.site', 'uuid', $source_storage->read('system.site')['uuid']);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this modify the UUID from the source config (i.e. what's on disk)? That is not a desirable behavior if the config is in version control - that config on disk should never change IMO. Since this option is only available when installing a fresh site, I think it's safe enough to change what's in the site instead of in the config directory.

Copy link
Author

@simesy simesy Dec 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it modifies the UUID in the active config of the newly installed site. I've been using @markdorison's variation on this and it has never modified the source yml file on disk.

@cweagans
Copy link

cweagans commented Dec 18, 2015

I think this should also change the default profile choice to Minimal if --config is passed. Minimal doesn't create any config entities by default, whereas Standard does (shortcut links is what bit us in the past). Minimal provides the most likely path to success when installing from config, so it makes sense to me to switch that default.

@simesy
Copy link
Author

simesy commented Dec 18, 2015

That seems like a sound idea. It also makes the whole process faster.

@simesy
Copy link
Author

simesy commented Dec 18, 2015

Commit added to default to 'minimal' per @cweagans suggestion.

@simesy
Copy link
Author

simesy commented Dec 18, 2015

Hmm. Regardless of what you default the installation to, the imported configuration (assuming it includes core.extensions) has to match the name of the install profile or you get:

Unable to uninstall the <em class="placeholder">Minimal</em> profile since it is the install profile.

@cweagans
Copy link

cweagans commented Dec 18, 2015

@simesy I think it's okay to default to Minimal. If they're using some other profile, they can change it by adding the profile argument.

@@ -55,7 +56,8 @@ function site_install_drush_command() {
'example-value' => 'directory_name',
),
'writable' => 'Make CMI and other dirs writable by both web and CLI users. Suitable for non Prod environments.',
'keep-config' => 'Keep CMI directories untouched. This preserves existing configuration.'
'keep-config' => 'Keep CMI directories untouched. This preserves existing configuration.',
Copy link
Member

@weitzman weitzman Dec 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options that are version specific should get altered in so they aren't valid or shown for other versions. see user_drush_help_alter() for example.

Copy link
Author

@simesy simesy Dec 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit just pushed removes --config for versions less than 8.

@weitzman
Copy link
Member

weitzman commented Dec 20, 2015

Left some minor comments. Would be good to hear that this actually works for folks.

@simesy
Copy link
Author

simesy commented Dec 21, 2015

To recap: All comments addressed. The call needs to be made about removing the 'minimal' profile part, I would suggest removing it unless we can replicate the problem it's mitigating.

@@ -55,7 +56,8 @@ function site_install_drush_command() {
'example-value' => 'directory_name',
),
'writable' => 'Make CMI and other dirs writable by both web and CLI users. Suitable for non Prod environments.',
'keep-config' => 'Keep CMI directories untouched. This preserves existing configuration.'
'keep-config' => 'Keep CMI directories untouched. This preserves existing configuration.',
'config' => 'After installation, override all configuration and UUIDs that were generated during installation with config from this file path.'
Copy link
Member

@weitzman weitzman Dec 23, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Update site UUID and then import configuration from this path."

@simesy
Copy link
Author

simesy commented Dec 23, 2015

Resolved comments in last commit.

@weitzman weitzman closed this in b0fd9a3 Dec 23, 2015
@weitzman
Copy link
Member

weitzman commented Dec 23, 2015

Squashed and merged. Thanks much.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Dec 25, 2015

This looks like it will be a good and useful feature, but we are not done yet; the new site-install option --config conflicts with the existing Drush global --config option, so folks like me who set --config cannot run site-install.

This local option needs to be renamed to something else.

greg-1-anderson added a commit that referenced this pull request Dec 25, 2015
Suggested fix for #1875: rename --config to --config-dir
@simesy
Copy link
Author

simesy commented Dec 25, 2015

Thanks greg!

@alexpott
Copy link
Contributor

alexpott commented Jan 9, 2016

There are quite a few gotcha with this approach. If the install profiles on the source config and the drush install don't match then there are issues. Also the install language is interesting - I think it should match the site default language from system.site. To be honest I really wish we all got behind https://www.drupal.org/project/config_installer since this is doing exactly what people want, does it without installing incorrect config first, and has tests.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Jan 9, 2016

I believe that the config installer works with drush site-install.

It sounds like the implementation of config_installer is currently more robust than what we have here, but some folks prefer the DX of the --config. Maybe some work could be done here to ensure consistency between installation variables and their corresponding configuration values. I don't know that it's a problem to install minimal profile first.

I think that the ideal solution, for the future would be to take the operation of the config_installer and make it part of the core installation code rather than having it be a separate installation profile. Until then, what we have is just a choice between different workarounds.

@simesy
Copy link
Author

simesy commented Jan 13, 2016

@alexpott To be honest I really wish we all got behind config_installer
I've passed over this solution because the projects I'm working on already use custom install profiles.

@dgtlmoon
Copy link

dgtlmoon commented Apr 25, 2016

Some help for those just catching up here.

So I've got a site profile which installs fine with ../vendor/bin/drush site-install my_profile and I've already exported the configs with ../vendor/bin/drush config-export.

however when I run ../vendor/bin/drush site-install my_profile --config-dir=../config

Installation complete.  User name: admin  User password: YdkMThXM9B                                                                                                                             [ok]
No config value specified.                                                                                                                                                                   [error]
Invalid argument supplied for foreach() DrupalBoot.php:240                                                                                                                                 [warning]
 Collection  Config                                    Operation 
             views.view.taxonomy_term                  delete    
             views.view.frontpage                      delete    
...
             core.date_format.html_datetime            delete    
             core.date_format.html_date                delete    
             core.date_format.fallback                 delete    
 sync        core.date_format.fallback                 create    
 sync        core.date_format.html_date                create    
 sync        core.date_format.html_datetime            create    
 sync        core.date_format.html_month               create    
...
Import the listed configuration changes? (y/n): y
Drupal\Core\Config\ConfigImporterException: There were errors validating the config synchronization. in Drupal\Core\Config\ConfigImporter->validate() (line 725 of                           [error]
/var/www/site/web/core/lib/Drupal/Core/Config/ConfigImporter.php).
The import failed due for the following reasons:                                                                                                                                             [error]
This import is empty and if applied would delete all of your configuration, so has been rejected.

This error does not really mean anything to me, where am I going wrong?

@dgtlmoon
Copy link

dgtlmoon commented Apr 25, 2016

@simesy have you seen https://www.drupal.org/node/2642398 ?

@fluxsauce
Copy link
Contributor

fluxsauce commented May 6, 2016

@dgtlmoon The practical solution is to use https://www.drupal.org/project/config_installer

For a drush.make.yml:

projects:
  config_installer:
    type: "profile"
    version: "1.3"

Installation:

drush site-install config_installer --config-dir=PATH/TO/config/base --yes

@marcelovani
Copy link

marcelovani commented Sep 5, 2016

I need to enable my own profile and import config from the repo to build a fresh site from code.

i.e. drush site-install my_profile --config-dir=PATH/TO/config/base --yes

How can I use config_installer to import the config? It is not possible to have two profiles enabled at the same time.

@marcelovani
Copy link

marcelovani commented Sep 6, 2016

I think I figured out why it wasn't working and how config_installer knows that I want my_profile enabled.

Basically core.extension.yml is what defines which profile will be enabled.

I had an entry for standard: 0 which I changed to my_profile: 0

Now it works.

I can only make it work using the UI. When using drush, I get an error saying that the folder config/base is empty.

mikeker pushed a commit to mikeker/drush that referenced this pull request Aug 10, 2017
mikeker pushed a commit to mikeker/drush that referenced this pull request Aug 10, 2017
…h existing Drush global --config option. Rename the new site-install option to --config-dir.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants