Problem/Motivation

Migrations are configuration entities. Why?

  • They get the config entity query-ability
  • They get the discovery and installation
  • They get config overrides (this is used in the Drush runner)
  • There might be a migrate builder with a UI

But:

  • They have no lifecycle - deleting a migration config entity through the API outside of uninstalling migrate_drupal does not really make sense. Config entities have lots of code to manage them through their life and migration uses none of this.
  • They are never configured - either by the API or UI
  • They pollute active configuration for no reason - there is no runtime requirement to access migrations - they exist for a use case that covers the 0.0000001% life of a site.
  • They pollute config export - putting them into your site config git repo - does that make sense
  • They pollute config import - they're only ever going to show here as an addition or deletion - just noise.
  • They have the overhead of config schema for translating one thing - the label
  • Config overrides are a double edge sword - there are many many things in a migration that make no sense to override

This issue would make #2460529: Migrations need to use the configuration entity dependency system redundant - which is the reason I started this issue.

Proposed resolution

Remove use of ConfigStorage and ConfigEntityBase in the migration classes. Use a YAML file reader that is able to read migration YAML files from another location.

Remaining tasks

  • Agree (fight)
  • Decide to go ahead or not
  • Review
  • Commit

User interface changes

None

API changes

Not as many as you would think

Files: 
CommentFileSizeAuthor
#6 2462233.6.patch56.22 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 90,159 pass(es). View
#6 1-6-interdiff.txt8.6 KBalexpott
#1 2462233.1.patch48.14 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 90,159 pass(es). View

Comments

alexpott’s picture

Status: Active » Needs review
Issue tags: +Configurables, +Configuration system
FileSize
48.14 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 90,159 pass(es). View

Initial patch which seems to work.

Still need to address query-ability and overrides.

Gábor Hojtsy’s picture

I think the main reason the migrate team was using config entities was to make it easier for the contrib migrate UI to be built. If migrations in core are not to be config entities, there at least need to be a plugin API so they can *also* be config entities so they can be configured, I guess, to satisfy that. But either way actual feedback from actual migrate owners would be good :)

alexpott’s picture

This does not stop a UI being built. Given the flexibility of the entity system it would not be hard for the UI to extend the loading of migrations to include its own configurable version (if it chose to make them config entities). But tbh I don't think that it is such a great a idea - a UI could still spit out the yml to be placed in a migrations directory. Designing the migration will be the hard part. Putting a file in a directory will be simple after that.

mikeryan’s picture

No, no, no.

The problems here are not inherent to migrations being configuration entities - they're due to the unique migrate_drupal approach of putting incomplete default configurations into active configuration. This is not how "normal" migrations will be done, and I'm not fond of migrate_drupal doing it - if anything, we should find an alternative for migrate_drupal, not throw out the whole thing. See #2202475: Registering/instantiating migrations for my previous commentary.

They have no lifecycle - deleting a migration config entity through the API outside of uninstalling migrate_drupal does not really make sense. Config entities have lots of code to manage them through their life and migration uses none of this.

Purely a migrate_drupal thing - most migrations will have a lifecycle, created/configured through a UI, deletable once a migration is complete.

They are never configured - either by the API or UI

They will be - even on Drupal 7, where it's a pain in the ass, we have a field mapping editor allowing configuration. The best part of moving Migrate to D8 is that it enables far more flexible configuration than is practical today.

They pollute active configuration for no reason - there is no runtime requirement to access migrations - they exist for a use case that covers the 0.0000001% life of a site.

Yes, migrate_drupal does this. Let's deal with it.

They pollute config export - putting them into your site config git repo - does that make sense

It will. In a typical custom migration project, you'll develop the migration configuration in a development environment to ultimately be deployed to production and run for real.

They pollute config import - they're only ever going to show here as an addition or deletion - just noise.

Not quite clear on the issue here.

They have the overhead of config schema for translating one thing - the label

There's more to config schema than translation, isn't there? And ultimately we will have more UI sugar around migration.

Config overrides are a double edge sword - there are many many things in a migration that make no sense to override

Well, that will vary according to the migration use case. Ultimately I hope to see rich UIs where migrations are built entirely from scratch, as well as canned migrations that are never altered.

mikeryan’s picture

So, migrate_drupal... One thought I had was that its migration configuration goes into something like config/templates and is not automatically installed into active configuration. Instead, when one uses the upgrade UI and adds the necessary information to actually run the migrations (source DB credentials), those templates are read and merged with the provided configuration, and at that point saved into active configuration as usable entities. This is actually pretty much what migrate_upgrade does right now - it gathers the credentials, identifies the source Drupal version, loads the relevant migrations from active configuration, and resaves them with the credentials added. The only difference with the template approach is it would instead read the templates, and active configuration would not be cluttered with them until they're actually used.

alexpott’s picture

FileSize
8.6 KB
56.22 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 90,159 pass(es). View

As we add more migrations for Drupal versions the number of migrations is only going to multiply. The template idea might work - but then I think we shouldn't put them in config/template because that implies all config can be templated. The "they are never configured by the API or UI" refers to the current state of HEAD. Configuration is one way of deploying things between environments. Code is another way. There is nothing in this patch that stops rich UIs being developed.

Here's a patch that adds query-ability. Needs tests - manually tested and it works well.

mikeryan’s picture

I still do not see any reason migrations should not be (or at least use) configuration entities. Please forget about what migrate_drupal is doing - it is a special snowflake, not reflective of how 99.99% of other migrations will work (nor a model of how they *should* work), and I agree that flooding the active configuration with unused/unusable migrations is the wrong approach. This is a reason to fix migrate_drupal, not to refactor migrate.

Configuration is one way of deploying things between environments. Code is another way.

Code is an inferior way. We've been configuring migrations in code in D6 and D7 and no mas, we're ready to configure them in configuration.

There is nothing in this patch that stops rich UIs being developed.

It won't stop them (after all, you can develop anything you want from scratch in contrib), but it'll slow them down as they'll have to re-implement the things they get for free as configuration.

mikeryan’s picture

For more background, I describe some typical migration scenarios at https://docs.google.com/a/acquia.com/document/d/1wmClshzFRRXn361-Yb7oh7A....

benjy’s picture

I agree that having 200 migration config entities is certainly a fair chunk of pollution, that's a valid argument but the counter is, disable migrate_drupal. You shouldn't ever have it enabled in production anyway after you've done the the migrations.

As I think Mike has said above, the blocker is the UI builder and how that would work. We want to build a UI that understands the source and destination data and provides a UI for developers to click together a migration. From there, you need to export that migration and likely deploy it somewhere. We don't want the UI writing to your codebase and if we write it to files it can't be deployed without some manual intervention. I think CMI solves this problem for us perfectly?

I'd also like to note that we now have a few chunks from entity query and ConfigEntityBase in migrate, I don't like the idea of maintaining such similar code in Migrate personally.

dasjo’s picture

I think deploying migration configurations using CMI is something I would have expected the system to provide me with, so I'm not sure if the right solution would be to put migrations in code rather than config. A migration doesn't necessarily have to be a one-time action, so I could imagine enabling migrate, running some migrations, deactivating it and repeating the process.

andypost’s picture

Maybe use the same approach that we have for "breakpoints" in core? they were entities and then became just a plain YML files as patch suggests

benjy’s picture

Status: Needs review » Postponed

I think we should postpone this on #2463909: Migrations should support non-installed default configurations (templates) I hope that becomes our solution to this otherwise, we at least need to explore it first.

dawehner’s picture

I'll encourage everyone to have a look at the actual patch.

  • Things are still an entity
  • Things are still queryable

The only thing which is missing is the writeability. Keep though in mind, that contrib can always swatch out the storage layer and even call out to config entities,
or something else. If you think about a UI though, that aspect of the problem space is the most minor ones: You need to figure out, how to make things understandable,
you need to figure out, how to build up the yaml configuration properly, the only thing which config entities gives you is to be able to save to disc. Putting things
directly into files gives us deployability, that is for sure.

Core should start to not solve every usecase but rather make the APIs flexible enough, so that contrib can deal with it later, if needed.
Contrib can now swap out stuff:

  • Store some of the migrations as config entity
  • Disable/toggle migrations by storing the information in state, for example

So personally I don't see the advantage at all to use configuration entities.

Beside that its odd that core optimizes for a usecase I think not even the majority of people will use.

mikeryan’s picture

One thing that's missing generally is guidelines on when to use configuration entities. What I can find includes:

  • The Configuration Entity API should be used to store multiple sets of configuration - for example node types, views, vocabularies, and fields. (https://www.drupal.org/developing/api/8/configuration)
  • These are used to store information that users can create and destroy and your code will continue to work fine whether there are 0 or 10+. Example: image styles. (https://www.drupal.org/node/2120523)
  • The main difference between configuration entities and Drupal 7-style content entities is that configuration entities use the configuration system for storage, rather than the database. They differ from plain configuration in that they are typically user-created, and often need to fire and respond to hooks. (https://www.drupal.org/node/1818734)
  • In contrast to the simple configuration settings described in the previous section, if your module allows users to create zero or more items (where "items" are things like content type definitions, view definitions, and the like), then you need to define a configuration entity type to store your configuration. (https://api.drupal.org/api/drupal/core!modules!system!core.api.php/group...)

To me, this all sounds (like it has since configuration entities came into being) natural for migration. So, if we were to write a guide on when and when not to use configuration entities, precisely what criteria would migrations fail? I still don't see why migrations on general principle aren't perfect examples of config entities, to me the one thing we're lacking is a good way to deal with the use case where a module is providing default partial configurations for migration (use case #2 at https://www.drupal.org/node/2456261#comment-9790273) as migrate_drupal is.

phenaproxima’s picture

I'm generally in agreement with @mikeryan.

Migrations, like views, are executable pieces of configuration. Like views, they can be executed more than once (rolling migrations). I can see several reasons why developers might want to keep them around in source control. Given that, I think it makes perfect sense for migrations to be, by default, standard config entities.

But, as has been stated, the critical thing here is that migrate_drupal is a bizarre case and it definitely should not install a bunch of pointless garbage into the active store. Ideally, migrate_drupal should never install anything, precisely because it's a one-time flash-in-the-pan type of thing. The template system would be an elegant and potentially reusable solution to that problem, so I'm +1 for that approach.

phenaproxima’s picture

Status: Postponed » Closed (won't fix)

As per discussion in our weekly IMP hangout, we decided that migrations will continue to be config entities, but properly leverage the config dependency system (#2460529: Migrations need to use the configuration entity dependency system). So this approach is moot.

Notes from the discussion, for reference: https://groups.drupal.org/node/469928#comment-1106583