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

Munge the uuids in sync dir before config-import #1635

Closed
wants to merge 1 commit into from

Conversation

@markdorison
Copy link

markdorison commented Sep 26, 2015

It was not previously possible to import a site configuration "over" a
freshly installed site. This is useful during development and in
automated testing environments when the database is not necessary as no
content is required (or desired).

Closes #1625

To Test

  1. drush site-install minimal
  2. Log in and configure some pieces of the site. Set the site title, enable some modules, etc.
  3. drush site-export
  4. drush site-install minimal
  5. drush config-import --force

You should be prompted as to whether you want to override the destination UUID. After this is complete, a config import will proceed.

screen shot 2015-09-26 at 14 21 28

It was not previously possible to import a site configuration "over" a
freshly installed site. This is useful during development and in
automated testing environments when the database is not necessary as no
content is required (or desired).

#1625
@weitzman
Copy link
Member

weitzman commented Sep 28, 2015

Whats the correct behavior for incoming config entities with this option? Those carry UUIds that wont match.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Sep 28, 2015

I almost brought up entity UUIDs previously. I was writing some functional tests, and wanted to test config import. My first idea was to store exported block configuration files with the tests, and copy them in to the staging directory and import them. The problem with this idea is that the exported configuration contains UUIDs that won't match with the UUIDs created during site install. The reason I moved away from this idea is that I was afraid that there would be cross-references between different entities using UUID. Seems that a force could make a mess of these, if only some of the files with cross-references were updated from a foreign site.

Perhaps this is expected behavior with --force ("I know what I am doing") it might be be helpful to log a warning every time a UUID is reset with the force option.

@markdorison
Copy link
Author

markdorison commented Sep 30, 2015

@weitzman @greg-1-anderson My intention was that it would replace all of the configuration entities so they in fact shouldn't match. This is reflected in the existing output which you can see part of in my screenshot that shows all of the existing configuration entities being deleted and new ones being created.

Please set me straight if I am missing something.

@weitzman
Copy link
Member

weitzman commented Sep 30, 2015

D8 has configuration dependencies so if you replace all the config entities, you wipe out all content types and fields and thus wipe all the content in the DB. We don't have a big enough warning for that! This is equivalent to doing a full site-install which means that --force would be a duplicate of https://www.drupal.org/node/1613424

@markdorison
Copy link
Author

markdorison commented Sep 30, 2015

@weitzman For the record, that is my intention with this functionality. I want to be able to spin up a site with the configuration files committed to version control without a database.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Sep 30, 2015

I ran into this use case myself when setting up CI for Drupal 8, and I have to say it is a little frustrating to not be able to commit configuration files and use them without the database that generated them. I got around the limitation for my tests by exporting the entire configuration set after site-install, and then modifying configuration via Behat + Drush, and then testing imports and exports from there. In my case, though, my purpose was only to test code; there was no specific site-under-test for me to worry about. To do functional tests on the configuration of a specific site, this workaround would be ineffective.

Perhaps this PR is the solution, although perhaps the name of the option, the help text for the option, and the warning message should be altered to indicate that the option means "import the configuration and erase the site content".

For use cases that are not CI-related, I think that features should be used instead.

@weitzman
Copy link
Member

weitzman commented Sep 30, 2015

Blowing away your site is not something I would ever expect config-import to do. This could be an option to site-install or better yet, use the Contrib project that built for exactly this use case. I linked to it in my last message. If that profile does not work with Drush (we got a report of this), then lets fix the problem or find a site-install based solution.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Sep 30, 2015

Handling this in site-install sounds like the best option, since that is really the only valid time that you'd want to force the config import to rewrite the UUIDs. The link above points to a postponed issue in core; is there a solution that we can use today? I did not see a reference to the contrib project, but I did not read everything in the referenced issue.

@weitzman
Copy link
Member

weitzman commented Sep 30, 2015

Lets close in favor of #1372

@weitzman weitzman closed this Sep 30, 2015
@alexpott
Copy link
Contributor

alexpott commented Oct 23, 2015

Another option is to update the staging config uuids. I do this like this:

/** @var \Drupal\Core\Config\FileStorage $sync */
$sync = \Drupal::service('config.storage.sync');
foreach ($sync->listAll() as $name) {
  $existing_config = \Drupal::configFactory()->get($name);
  if (!$existing_config->isNew() && ($uuid = $existing_config->get('uuid'))) {
    $data = $sync->read($name);
    $data['uuid'] = $uuid;
    $sync->write($name, $data);
  }
}

$core = $sync->read('core.extension');
unset($core['module']['standard']);
$core['module']['minimal'] = 1000;
$sync->write('core.extension', $core);
@weitzman
Copy link
Member

weitzman commented Oct 23, 2015

Oh, thats clever! Edit the files in the sync dir with so their uuids match those on the running site.

@weitzman weitzman reopened this Oct 23, 2015
@cweagans
Copy link

cweagans commented Oct 23, 2015

We're doing something pretty similar on our project. Excerpt from our makefile:

install-site:
    ./vendor/bin/drush --root=$(DIR)/docroot site-install minimal --yes

config-init:
    cat ./config/system.site.yml | grep uuid | tail -c +7 | head -c 36 | ./vendor/bin/drush --root=$(DIR)/docroot config-set -y system.site uuid -

config-import:
    ./vendor/bin/drush --root=$(DIR)/docroot config-import sync --yes

rebuild: install-site config-init config-import

I think it makes a lot more sense to put this in site-install than in config-import, because we're only ever doing this on the initial installation. I'd also point out that doing this with pretty much anything other than the minimal profile is likely going to fail. One example of a problem we ran into was Shortcuts. When we were installing with Standard, we'd get this on config-import:

Entities exist of type <em class="placeholder">Shortcut link</em> and
<em class="placeholder"></em> <em class="placeholder">Default</em>.
These entities need to be deleted before importing.

Given that, I'd suggest just making a separate command or flag or something, so that you can do: drush si --from-config=/path/to/config/files, which would install minimal, set the site UUID properly, and then import the config. That's not much different from what was suggested here earlier - the only difference is that Drush would just assume that it should install from Minimal if config is going to be imported

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Oct 23, 2015

At BADCamp, @alexpott stated pretty clearly that he felt that the config installer was the way and the future; however, this won't be in until 8.1.0.
I agree that site-install should handle config import without requiring any installer that is not packaged with core.

@weitzman weitzman changed the title Adds force option to drush config-import. Munge the uuids in sync dir before config-import Nov 3, 2015
@weitzman
Copy link
Member

weitzman commented Nov 3, 2015

Retitle based code from @alexpott above. This is exotic enough that it should be its own config-uuid command, not a flag on config-import.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Nov 3, 2015

I don't think this is so exotic. Apparently, the uuids are only in place to determine whether or not configuration was deleted and then re-created with the same name; all cross-references are done using data other than the uuids. Because of this, I think that config import --force makes a lot of sense: "I want these two sites to be the same site now." It would be much less discoverable (and ergo confusing) if it was split into two separate commands.

We could always warn and confirm when --force is used.

@weitzman
Copy link
Member

weitzman commented Nov 3, 2015

OK, makes sense. Anyone is welcome to submit a PR using @alexpott code

@alexpott
Copy link
Contributor

alexpott commented Nov 3, 2015

The one problem which does make it exotic are install profiles. If the incoming configuration has a different profile in core.extension (not so easy to figure out) then this command is going to need to deal with this.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Nov 3, 2015

@alexpott: Is this situation repairable (i.e., is the fixup limited to adjusting the installation profile recorded in settings.php), or are there deeper reasons why switching profiles won't work?

In any event, I think it would be a reasonable limitation if --force still failed with an error if the profile did not match.

@cweagans
Copy link

cweagans commented Nov 3, 2015

Is it unreasonable to say "If you're going to use Drush for importing config, you need to plan on using Minimal for the default installation everywhere"? The group of people that are keeping a big pile of config in their Git repo probably doesn't overlap very much (if at all) with the group of people that are building install profiles.

Alternatively, Drush can just complain loudly if the target site and the incoming config have profiles that don't match. I think that would also be a reasonable path forward here.

@alexpott
Copy link
Contributor

alexpott commented Nov 3, 2015

Yep there is a bigger reason - install profiles change where we look for modules - if a module only exists in the profile and we do this we've got problems

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Nov 3, 2015

@alexpott Yes, installation profiles change where Drupal looks for modules. Is updating the profile entry in settings.php sufficient to overcome this sort of change?

@alexpott
Copy link
Contributor

alexpott commented Nov 3, 2015

Yep it is. There are certain complications like if the starting profile has
modules. I'd suggest that you only allow swapping profiles if the profile
of the target site is minimal or standard before importing. That means you
know the site you are installing to has no custom modules in the profile.

On 3 November 2015 at 08:46, Greg Anderson notifications@github.com wrote:

@alexpott https://github.com/alexpott Yes, installation profiles change
where Drupal looks for modules. Is updating the profile entry in
settings.php sufficient to overcome this sort of change?


Reply to this email directly or view it on GitHub
#1635 (comment).

@simesy
Copy link

simesy commented Dec 1, 2015

There's a bit of talk about using the minimal profile, which I'm not. I merged this branch into my drush and did:

drush config-export sync
drush site-install PROFILE
drush config-import sync

This gives me the described errors, but I'm also getting the errors if I try to replicate using the minimal profile, so either this branch is stale (it's 9 weeks old?) or I've screwed up something.

@simesy
Copy link

simesy commented Dec 1, 2015

Use case: We have 30 sites. We're using install profiles to neatly namespace and delineate custom code from shared code. The team has no shared server resources (yet). The sites have no content (yet). Trying to do initial builds from code with no databases.

@simesy
Copy link

simesy commented Dec 1, 2015

Oh wait. --force might help. This works with my custom profile.

@simesy
Copy link

simesy commented Dec 2, 2015

Update. I've been working successfully with --force for a few days, with a custom install profile. Not just core config, display suite for example.

@simesy
Copy link

simesy commented Dec 2, 2015

For those looking for similar solutions in this area... https://www.drupal.org/project/config_installer

@weitzman
Copy link
Member

weitzman commented Dec 3, 2015

The approach in #1635 (comment) looks way more safe than the one in the PR. Edit the yaml files to match the site, not vice versa. Anyone is welcome to submit a new PR that does that.

@cweagans
Copy link

cweagans commented Dec 3, 2015

What difference does it make? You're still going to end up importing a bunch of foreign config over the top of what's already in the site.

@cweagans
Copy link

cweagans commented Dec 3, 2015

(Not arguing, btw. Genuinely curious as to why you see it that way)

@weitzman
Copy link
Member

weitzman commented Dec 3, 2015

@simesy
Copy link

simesy commented Dec 3, 2015

In my workflow this will result in noisy commits with unnecessary UUID changes in my config files, but I'm sort of re-appropriating the command for a different use case anyway...

@cweagans
Copy link

cweagans commented Dec 3, 2015

Yeah, I saw that. I'm just not sure why changing the config resolves that issue. We still end up blowing away all the config in the site in favor of what's on disk, but going this route (as @simesy said), we end up modifying the config for every instance of the site, so the site UUID will never be in sync. As an example, if you have 3 developers working locally + dev, stage, prod, the UUID in the config checked into git will change basically every time the config is imported, won't it?

Kind of thinking it would be better to not do this on config-import, and have it be a flag on site-install instead (since that's the only use case I can think of where one would want to do this). Something like drush si --from-config-dir="sync".

@weitzman
Copy link
Member

weitzman commented Dec 3, 2015

The problem isn't that config is overwritten. Thats a given, as you say. The problem is that the site's content will all get deleted as content depends on config entities for stuff like content types, fields, etc. Referential integrity is a bitch.

@cweagans
Copy link

cweagans commented Dec 3, 2015

@weitzman Right. So what's the difference between changing the UUID in the config to match the site, or changing the UUID in the site to match the config, specifically for the site content problem?

What do you think about doing this in site-install instead?

@weitzman
Copy link
Member

weitzman commented Dec 3, 2015

In one scenario all content gets deleted and in the other it doesn't (because the config entity uuids are not changing and this the incomping config is treated as an update, not a delete+insert)

I already suggested doing this as part of site-install. Any now myself and @simesy have linked to config_installer project. This issue is going in circles.

@cweagans
Copy link

cweagans commented Dec 3, 2015

@weitzman Oh, I see. I didn't read the code in #1635 (comment) closely enough. It's changing all of the UUIDs, not just the one in system.site.yml. I was misunderstanding the direction here. Sorry for the noise.

Also, the config_installer project is nice, but I don't think it should be required to install from config. @greg-1-anderson said the same in #1635 (comment). Maybe we can do both things? i.e. I can open another PR to add a --from-config option to site-install?

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Dec 3, 2015

I think it would be great is the site-install command could handle a forced config import without a custom installer. I think importing configuration during site-install is the ideal workflow for creating dev sites, where config changes might be committed back to the repository. A --force option on config-import would be handy for unit tests, but perhaps not necessary if site-install did the right thing, as subsequent re-imports would then have matching UUIDs.

@weitzman
Copy link
Member

weitzman commented Dec 3, 2015

Also, the config_installer project is nice, but I don't think it should be required to install from config.

if config_installer is headed toward core for 8.1.0, why don't folks just work on that? We are only 5 months from 8.1.0. We may or may not have Drush 8.1.0 before then

@cweagans
Copy link

cweagans commented Dec 3, 2015

config_installer, IMO, is not very useful until we have install profile inheritance. Even if config_installer is in core, I'd probably still continue to drush si minimal and import my config on top of it.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Dec 3, 2015

I would tend to think that it would not take very long to release a version of Drush with an updated site-install command, iff someone went to the effort to make one.

@simesy
Copy link

simesy commented Dec 3, 2015

OK I looked at the patch, I will work on a PR for updating the site-install tonight. I won't use a --force argument because there's no target to destroy. But since drush si doesn't currently take a source directory/label, I'd like to know...

# Do you want:
--use-config-source=/path/to/directory
# Or...
--use-config-dir=sync
# Or both.
@weitzman
Copy link
Member

weitzman commented Dec 13, 2015

I'm thinking that a simple --config option would do. Thanks.

@simesy
Copy link

simesy commented Dec 18, 2015

I've opened a separate pull request at #1875

@weitzman
Copy link
Member

weitzman commented Jan 8, 2016

OK, lets close this in favor of #1875

@weitzman weitzman closed this Jan 8, 2016
@weitzman
Copy link
Member

weitzman commented Jan 8, 2016

Woops, meant to say - lets close this as #1875 went in. Hurray!

@mriffault
Copy link

mriffault commented Apr 11, 2017

I managed to use @cweagans method provided above (a big THANKS to him!). For the entity issue (which I encountered on language entities), I have moved them in the config/install folder of my installation profile. And I use Drush CMI tools (which is awesome too!) to exclude this config from export and manage partial imports.

Now my build works fine, I can rebuild the website from scratch and import the configuration from code without using config_installer :)

Here is an example of my Makefile:

# @source https://github.com/previousnext/drush_cmi_tools/issues/5#issuecomment-250008389
APP_ROOT=$(CURDIR)/docroot
CONFIG_DIR=$(CURDIR)/config/sync
CONFIG_IGNORE=$(CURDIR)/config/config-ignore.yml
CONFIG_INSTALL=$(CURDIR)/config/install
CONFIG_SKIP_MODULES=devel

# Set DEL to 1 on make call to use the config deletion file (on import)
ifeq ($(DEL),1)
	CONFIG_DELETE=$(CURDIR)/config/config-delete.yml
	DELFLAG=--delete-list=$(CONFIG_DELETE)
else
	DELFLAG=
endif

install:
# The dash before drush will ignore errors and allow "rebuild" recipe to continue
# even if some non-critical errors occur during install (like sending the
# confirmation e-mail to the administrator)
	-drush si -y my_profile \
		-r $(APP_ROOT) \
		--db-url=mysql://db_user:db_pass@mysql:3306/db_name \
		--db-su=root \
		--db-su-pw=rootpasswd \
		--account-name=admin \
		--account-pass=admin \
		--account-mail=admin@test.com \
		--locale=fr \
		--site-name="My website" \
		--site-mail=admin@test.com

# We have to override site uuid to be able to import the config from a previous
# install.
# @source https://github.com/drush-ops/drush/pull/1635#issuecomment-150661519
init:
	cat $(CURDIR)/config/sync/system.site.yml | grep uuid | tail -c +7 | head -c 36 | drush -r $(APP_ROOT) config-set -y system.site uuid -

# You may encounter errors on importing "entities" config (like
# language.entity...), this kind of config information have to be excluded from
# the export using config-ignore.yml, and could only been provided during site
# installation in the config/install directory of my_profile.
import:
	drush -r $(APP_ROOT) cimy -y --skip-modules=$(CONFIG_SKIP_MODULES) --source=$(CONFIG_DIR) --install=$(CONFIG_INSTALL) $(DELFLAG)

export:
	drush -r $(APP_ROOT) cexy -y --skip-modules=$(CONFIG_SKIP_MODULES) --destination=$(CONFIG_DIR) --ignore-list=$(CONFIG_IGNORE)

rebuild: install init import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.