Suggested commit message:

Migrate in core #2125717 by mikeryan, chx, marvil07, bdone, jessehs, mpgeek, BTMash, fastangel, mongolito404, Moshe Weitzman, eliza411, YesCT, dawehner, cosmicdreams

Note to reviewers: you are doing yourself a huge disservice if you don't read https://drupal.org/node/2127611 the introduction handbook page first. It's short. And necessary.

Problem/Motivation

The current upgrade path is unmaintainable, broken, a bad idea in the first place.

Proposed resolution

Instead of pointing a D8 codebase to a D7 database and try to bend each other together until they work together to upgrade to a D8 database use a migration process. Documentation has been started at https://drupal.org/node/2127611 -- I do not submit fundamental patches without some documentation written first.

With migration, you can utilize a running Drupal site to create configuration and content based on another datastore. The benifits of this approach are many

  • We can evolve a datasource (of which a D7 database is just one kind) to a D8 datasource in a manner that doesn't destroy the original.
  • We can make this system resilent to failure and resumable
  • We can make this system friend to automated systems so that you can continuously migrate data from one source to another. So, migrate everything into a staging site and when going live have a minimal downtime to migrate since the last run. No more 24 hr downtimes to upgrade drupal.org!
  • We can provide a flexible architecture that future migration efforts (from other datasources) can extend
  • We can finally ship a capability to migrate data from D6, D7, and other systems into Drupal 8.

Focus of this patch

The current patch implements the migrate framework full of interfaces, one implementation for each, unit tests, a complete migration with a simpletest. For unit testing the database reading sources, an in-memory DBTNG Select driver is provided.

In future patches the foundational work here is not expected to change, we expect additions; possibly the actual migration entities and tests will be moved to another module so it can be disabled once it's done. That'll be much easier once we switch from a module-in-a-sandbox to a core-fork-in-a-sandbox.

The DBTNG driver will be extended to a full driver with Insert/Update/Merge support and moved under Database/memory.

Note to developers familar with the Migrate module

If you were familiar with Drupal 7 migrate module you will find very, very familiar pieces of code. We are not rewriting migrate, we are rearranging it into a D8 structure where migrations are configuration entities and almost every piece of logic is in a plugin: a source, a process, a destination or an id map plugin.

Remaining tasks

Review this.

Past patch #1 we already have a lot more sources (practically all of D6 is covered, all of them unit tested). Each will be introduced with a real migration in a separate patch once we have the adequate destination too. These will be smaller patches, one bigger patch will be the batch of variables-to-config similar to the migration introduce here however that one will be next to trivial to review.

User interface changes

Not in this patch. Future work outlined here: https://groups.drupal.org/node/356283 describe our thoughts around providing a user interface. The UI is not a priority, by far, core will only ship with a very simple one, Bojhan offered help with this one.

API changes

We are adding a migrate API.

Additional impacts / effects

In implementing the migration system as a series of plugins for importing data from outside sources to internal systems we open the door to a great many data intensive applications. The source of the data simply needs to contain data relevant to Drupal. A complete migration system simply needs to provide a mechanism to extract the data from the source, transform the data to a form that Drupal understands and loads the data into Drupal. These are seperate and independent systems. In short, contrib space will have a very productive development cycle where this system can be made to solve many different data-oriented problems.

Files: 
CommentFileSizeAuthor
#63 2125717_62.patch164.62 KBchx
PASSED: [[SimpleTest]]: [MySQL] 59,031 pass(es).
[ View ]
#63 interdiff.txt1.87 KBchx
#61 2125717_60.patch164.57 KBchx
FAILED: [[SimpleTest]]: [MySQL] 58,766 pass(es), 510 fail(s), and 106 exception(s).
[ View ]
#61 interdiff.txt2.99 KBchx

Comments

chx’s picture

Status:Needs review» Needs work

The last submitted patch, migrate_1.patch, failed testing.

chx’s picture

Status:Needs work» Needs review
StatusFileSize
new131.63 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]

Status:Needs review» Needs work

The last submitted patch, 3: 2125717_3.patch, failed testing.

chx’s picture

StatusFileSize
new133.67 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]
chx’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 5: 2125717_4.patch, failed testing.

chx’s picture

StatusFileSize
new135.87 KB
PASSED: [[SimpleTest]]: [MySQL] 60,108 pass(es).
[ View ]
chx’s picture

Note that all these patches are only because I constantly leave out this or that class because we have so much more and I tried to keep the patch small :/

chx’s picture

Status:Needs work» Needs review
cosmicdreams’s picture

Issue summary:View changes
cosmicdreams’s picture

+++ b/core/modules/migrate/lib/Drupal/migrate/Entity/MigrationInterface.php
@@ -0,0 +1,77 @@
+/**

+++ b/core/modules/migrate/lib/Drupal/migrate/Plugin/MigratePluginManager.php
@@ -0,0 +1,60 @@
+ * Contains \Drupal\migrate\MigraterPluginManager.
...
+   * Constructs a MigraterPluginManager object.

The class for this file is MigratePluginManager, not MigraterPluginManager. Typo.

chx’s picture

Issue summary:View changes
chx’s picture

Issue summary:View changes
chx’s picture

Assigned:chx» Unassigned
cosmicdreams’s picture

Issue summary:View changes
aspilicious’s picture

I understand your feeling about doxygens. But if you're not willing to tackle it yourself (understandable) than you should try to gather some people to fix it. The current state is not so good. Lot's of functions don't have any documentation on it. I don't say everything should be *perfect* but we shouldn't commit this as we demanded better docs on patches way bigger than this one...

I have the feeling there are a bunch of people willing to help on this so it shouldn't be hard to find the right person.

YesCT’s picture

I'll take a first stab at docs stuff.

At the same time, others can do reviews besides the docs.

chx’s picture

Note: it's not just about doxygen. I am very glad, really glad that people are picking up the doxygen, that's great but my issue is with "some patches are more equal than others". In other words, I would like to get the same treatment as https://drupal.org/node/1874530 did. When that happens, there's no point in even trying to get a patch in. Finalizing the doxygen is superb irregardless of that.

chx’s picture

Issue summary:View changes
chx’s picture

Issue summary:View changes
dawehner’s picture

+++ b/core/modules/migrate/tests/Drupal/migrate/Tests/MigrateTestCase.php
@@ -0,0 +1,65 @@
+abstract class MigrateTestCase extends UnitTestCase {
...
+  /**
+   * Provide meta information about this battery of tests.
+   */
+  public static function getInfo() {
+    return array(
+      'name' => 'Migrate test',
+      'description' => 'Tests for migrate plugin.',
+      'group' => 'Migrate',
+    );

Not really needed

cosmicdreams’s picture

@YesCT, I started working on doxygen here: #2125083: Remove unused $keep variable from migrate sources

YesCT’s picture

I wanted to attach files, but the page wont load. hope this saves. :)
it is just a diff on my changes to the DrupalMessage class.
seemed like drupalmessage would be general for anyone, but extended the migrate message interface. guessing it was named drupal message since it used drupal set message, not cause it was meant to be generic. anyway, posting this for preliminary feedback before I get too far into it.

http://privatepaste.com/af5ee6f77e

YesCT’s picture

I would prefer the two be committed together and I'm not sure working on docs in another issue is a good idea.

(for example, when I went to do the docs, I changed more than just docs. I renamed a method.)

I dont really know what the best workflow is here, but I lean toward just keeping them in the same issue.

cosmicdreams’s picture

migrate_instrument_start() and migrate_instrument_stop() both appear to be undefined.

Status:Needs review» Needs work

The last submitted patch, migrate_1.patch, failed testing.

cosmicdreams’s picture

Status:Needs work» Needs review

8: 2125717_5.patch queued for re-testing.

chx’s picture

> I dont really know what the best workflow is here,

I do. Use the sandbox. I already found the broken file #26 was alluding to and removed it from the sandbox (it wasn't in this issue). Sandbox moves at a great pace. Core never does. Except for the patches that are more equal than others.

Status:Needs review» Needs work

The last submitted patch, migrate_1.patch, failed testing.

YesCT’s picture

OK. I'm willing to try something new. using the sandbox, pushed the changes from #24, which can be seen http://drupalcode.org/sandbox/chx/2105305.git/commit/13f7703 . And, I guess I'll comment on #2125083: Remove unused $keep variable from migrate sources as I proceed.

I'm not sure about having multiple issues regarding the same sandbox, but I guess it can serve to try and focus the commenting on particular subjects. [edit: OH! duh, I see the other issue is in the imp project/sandbox issue queue, and this is the core issue. Sorry.]

For reviewers, this means you should clone from the sandbox, not look at the patch. Something that has worked pretty well for config_translation is to post a new patch every few days to make it easier to get outside eyes on.

chx’s picture

Status:Needs work» Needs review

Some testbot issues dragged up an old patch (not that they were breaking for real; it's just a few git add missing).

chx’s picture

Issue summary:View changes

With dawehner volunteering to review and YesCT and cosmicdreams working on the doxygen I am revoking my rant on the core process but this does not mean I am happy with the current state of affairs, not at all.

chx’s picture

StatusFileSize
new19.94 KB
new137.34 KB
FAILED: [[SimpleTest]]: [MySQL] 60,052 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
chx’s picture

StatusFileSize
new137.5 KB
FAILED: [[SimpleTest]]: [MySQL] 60,021 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new20.77 KB

Status:Needs review» Needs work

The last submitted patch, 34: 2125717_6.patch, failed testing.

chx’s picture

Issue summary:View changes
Status:Needs work» Needs review
StatusFileSize
new139.55 KB
PASSED: [[SimpleTest]]: [MySQL] 60,165 pass(es).
[ View ]
new34.87 KB

This version contains the breakup of PropertyMap into CopyFromSource, DefaultValue (and sandbox only, ValueFromMigrations and soon HandleSqlDuplications). I will discuss the fate of the separator with mikeryan.

chx’s picture

Issue summary:View changes
chx’s picture

Issue summary:View changes
chx’s picture

StatusFileSize
new150.83 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

Process syntax revamped, closer to D7 now, documentation is also updated. Lots of doxygen added.

chx’s picture

Issue summary:View changes

Updating commit message after realizing two nicks are the same person.

Status:Needs review» Needs work

The last submitted patch, 40: 2125717_40.patch, failed testing.

chx’s picture

Status:Needs work» Needs review
StatusFileSize
new151.78 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

Broken reroll, nothing really broken. Added new hooks, documented.

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

Yes, the patch has a lot of TODOs but this patch is so important to get a proper upgrade path, which is critical
both for the success of Drupal 8 and Drupal in general. By letting this in the current state we will have a lot of more potential contributors
to all the upgrade migrations we need.

  1. +++ b/core/modules/migrate/lib/Drupal/migrate/MigrateExecutable.php
    @@ -0,0 +1,526 @@
    +  public function getSource() {
    ...
    +      $this->source = new Source($this->migration, $this);
    +    }
    +    return $this->source;
    +  }
    +
    ...
    +   * The rollback action to be saved for the current row.
    +   *
    +   * @var int
    +   */
    +  public $rollbackAction;

    Note: we are mixing up the order of properties and methods here.

  2. +++ b/core/modules/migrate/lib/Drupal/migrate/MigrateExecutable.php
    @@ -0,0 +1,526 @@
    +  public function import() {

    Given that this is probably the enter of all debugging for people, we should concentrate to document that properly in one of the next core issues.

  3. +++ b/core/modules/migrate/lib/Drupal/migrate/MigrateMessageInterface.php
    @@ -0,0 +1,19 @@
    +interface MigrateMessageInterface {

    At least a little bit of documentation in this file would be cool

  4. +++ b/core/modules/migrate/lib/Drupal/migrate/Plugin/MigrateIdMapInterface.php
    @@ -0,0 +1,169 @@
    +   * @TODO: YUCK THIS IS SQL BOUND!
    ...
    +  public function getQualifiedMapTable();

    I guess this is just called by some sql specific code somewhere? In a future patch you could try to move that to a sql specific interface.

  5. +++ b/core/modules/migrate/lib/Drupal/migrate/Plugin/MigrateProcessInterface.php
    @@ -0,0 +1,36 @@
    +   * @param $value
    ...
    +   * @param MigrateExecutable
    ...
    +   * @param Row $row

    Longterm nitpick: use absolute class references.

  6. +++ b/core/modules/migrate/lib/Drupal/migrate/Plugin/MigrateSourceInterface.php
    @@ -0,0 +1,45 @@
    +
    +  public function __toString();

    Future improvements: We could think about adding some kind of arbitary method for it. There was a lot of interesting dicussion about this on http://pooteeweet.org/blog/2231

  7. +++ b/core/modules/migrate/lib/Drupal/migrate/Plugin/migrate/destination/Config.php
    @@ -0,0 +1,87 @@
    + * Persist data to the config system.

    It seems to be that someone copy and pasted some documentation

  8. +++ b/core/modules/migrate/lib/Drupal/migrate/Plugin/migrate/destination/DestinationBase.php
    @@ -0,0 +1,53 @@
    +

    Future improvements: We should document why all this methods is implemented here with an empty implementation

  9. +++ b/core/modules/migrate/lib/Drupal/migrate/Plugin/migrate/process/Get.php
    @@ -0,0 +1,55 @@
    + * Contains \Drupal\migrate\Plugin\migrate\process\CopyFromSource.
    ...
    + * This plugin copies from the source to the destination.
    ...
    + * @PluginId("get")
    ...
    +class Get extends PluginBase implements MigrateProcessInterface {

    Future improvements: Documentat that one better ... i guess the previous comment was leftover from CopyFromSource.

  10. +++ b/core/modules/migrate/lib/Drupal/migrate/Tests/MigrateTestBase.php
    @@ -0,0 +1,55 @@
    +class MigrateTestBase extends WebTestBase {

    This is really awesome, as it will allow a lot of additional future tests.

chx’s picture

As a generic note: most of the properties on MigrateExecutable will, in due course, move to Migration where they belong. MigrateExecutable was brought up to the point where it can run but it is still very D7-y here and there. This will change. Getting a migration up and running, getting the plugin structure in is much more important.

I plan to submit the second core patch in a few days introducing the entity destination and with that we can begin to audit all the D6->D7 updates... and file issues for all of them for the community to help. And I tell you, none of that depends on MigrateExecutable . It depends on source, process and destination plugins. The rest is helpful but not crucial.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 43: 2125717_43.patch, failed testing.

chx’s picture

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new152.36 KB
PASSED: [[SimpleTest]]: [MySQL] 60,368 pass(es).
[ View ]

Eh, just TestSource.php was left out. And that is hardly a significant one. I have applied this patch against a clean checkout and ran phpunit; it passed.

chx’s picture

StatusFileSize
new153.53 KB
FAILED: [[SimpleTest]]: [MySQL] 60,425 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new27.44 KB

This is where we are at just before patch #2 -- that patch will feature more process plugins and our first working entity migration, filter formats. Better method names, migration specific prepare row hook, more powerful FakeSelect, doxygen fixes are the highlights of this version. Also added is the system cron migration cos it is in the same test class as system site and I couldn't bother with separating them.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 48: 2125717_48.patch, failed testing.

alexpott’s picture

+++ b/core/modules/migrate/lib/Drupal/migrate/Tests/MigrateTestBase.php
@@ -0,0 +1,59 @@
+    foreach ($files as $file) {
+      if (substr($file, -3) == '.gz') {
+        $file = "compress.zlib://$file";
+        require $file;
+      }
+      $class = 'Drupal\migrate\Tests\Dump\\' . basename($file, '.php');
+      $class::load($database);
+    }

I'm concerned that hardcoding the namespace means that only the migrate module with be able to provide tests.

chx’s picture

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new155.28 KB
PASSED: [[SimpleTest]]: [MySQL] 60,384 pass(es).
[ View ]
new29.76 KB

Again I left out a file (the system cron dump). Also fixed the problem pointed out by alexpott where only migrate can provide a test by parsing the namespace out of the file. Trying to figure it from the file name is too hard. It's one line of code added for namespace:

<?php
      $namespaces
= preg_filter('/^namespace (.*);/', '\1', file($file));
     
// trim() is necessary to remove the line break file() keeps.
     
$class = trim(reset($namespaces)) . '\\' . basename($file, '.php');
?>

interdiff is still against 46.

chx’s picture

alexpott noted that the idmap interface and the sql class is not quite nice documentation wise, still too much key vs id and so on. Well, I didn't want idmap to part of patch #1 but webchick wanted a complete migration so idmap is merely brought up to the level where it works.

If this patch does not get in within a week I will halt working on migrate. And here I am unfollowing the issue.

chx’s picture

*sigh* I can't even do that. OK I will continue no matter what. The unfairness kills me.

chx’s picture

Status:Reviewed & tested by the community» Needs work

OK I give in. I am not WSCCI to get anything committed. Got it. I concede. I will get the doxygen in a lickety split condition and spend as many months necessary rerolling and bikeshedding as necessary.

dawehner’s picture

If you want to complain about unfairness than you should complain about VDC.

chx’s picture

Status:Needs work» Needs review
StatusFileSize
new67.86 KB
new160.38 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]

A "few" doxygen fixes, variable renames and the fake database rehauled a little. Little. The interdiff size is not even half of the patch size.

Status:Needs review» Needs work

The last submitted patch, 56: 2125717_55.patch, failed testing.

chx’s picture

Status:Needs work» Needs review
StatusFileSize
new69.51 KB
new162.65 KB
FAILED: [[SimpleTest]]: [MySQL] 59,018 pass(es), 3 fail(s), and 4 exception(s).
[ View ]

It's the 0 byte migrate.module... interdiff against 51.

Status:Needs review» Needs work

The last submitted patch, 58: 2125717_55.patch, failed testing.

dawehner’s picture

I really think that we should rather improve the nitpicks you could apply everywhere over time. We should care more about the features
rather than the potential dept opened by that code. We will always improve things.

Here are just some nitpicks, which I would have not posted when the patch would have been green.

  1. +++ b/core/modules/migrate/migrate.api.php
    @@ -0,0 +1,37 @@
    +use Drupal\migrate\Entity\MigrationInterface;
    +use Drupal\migrate\Plugin\MigrateSourceInterface;
    +use Drupal\migrate\Row;

    As all the other ones, not important, but usually we use absolute classes in .api.php files.

  2. +++ b/core/modules/migrate/tests/Drupal/migrate/Tests/D6VariableTest.php
    @@ -0,0 +1,74 @@
    +class D6VariableTest extends MigrateSqlSourceTestCase {

    Missing @group Drupal

  3. +++ b/core/modules/migrate/tests/Drupal/migrate/Tests/process/GetTest.php
    @@ -0,0 +1,89 @@
    +/**
    + * @group migrate
    + */
    +class GetTest extends MigrateProcessTestCase {

    +++ b/core/modules/migrate/tests/Drupal/migrate/Tests/process/MigrateProcessTestCase.php
    @@ -0,0 +1,38 @@
    +
    +class MigrateProcessTestCase extends MigrateTestCase {

    Missing @group Drupal, @group Migrate

chx’s picture

Status:Needs work» Needs review
StatusFileSize
new2.99 KB
new164.57 KB
FAILED: [[SimpleTest]]: [MySQL] 58,766 pass(es), 510 fail(s), and 106 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, 61: 2125717_60.patch, failed testing.

chx’s picture

Status:Needs work» Needs review
StatusFileSize
new1.87 KB
new164.62 KB
PASSED: [[SimpleTest]]: [MySQL] 59,031 pass(es).
[ View ]

Hrm, timplunkett warned me of the trailing comma, didn't he.

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

Yeah, it is green again!

chx’s picture

Issue summary:View changes
catch’s picture

Assigned:Unassigned» Dries

dreditor is playing up for me, fortunately I didn't get too far with the patch before trying to paste.

I really like the YAML format for migrations, it's very self-documenting looking at the variables one and having the process stuff in there is fantastic.

Lots of nitpicks which got lost by dreditor, I haven't done an in-depth review.

With the caveat that I've not reviewed properly, I do agree this should go in ASAP. By definition this doesn't block the release, and it's going to have to be exempted from API changes at least right up until release candidate most likely. So we only gain visibility and the ability to split up work by having it in core.

Explicitly moving over to Dries since this is brand new stuff. If dreditor doesn't crash on me I'll try to review later, but don't hold it up on me doing so.

Dries’s picture

Assigned:Dries» Unassigned
Priority:Critical» Major
Status:Reviewed & tested by the community» Active
Issue tags:+Needs change record

I spent about 90 minutes on this patch. It's not nearly enough time to understand everything that is going on in the patch. Like catch, I saw a lot of small problems in the patch (e.g. documentation mistakes). I was just focused on trying to understand the basic architecture and approach so I didn't document these. Even with the limited time, I feel like I understand the overall approach taken and feel comfortable with going in this direction.

My initial reaction is that the YML files are an interesting and clever way of defining migration steps. It looks very verbose though; a lot of information seems to be repeated in the YML files (for example, in the source portion and again in the process portion). It is a DX concern I have, but doesn't have to hold up this patch. I feel it would be easier to get this patch committed, and iterate on this later.

Committed and pushed to 8.x. Great work!

PS: Absolutely love the Migration API documentation at https://drupal.org/node/2127611. It would be great to document the "map" process plugin, since it is used in the iterator plugin example, but without its own docs, made it hard to understand the example.

plach’s picture

Title:Migrate in core: patch #1» Change notice: Migrate in core: patch #1

woot!

chx’s picture

I am not sure what change notice should I write. We didn't write a change notice saying Views got added to core (here are all the Views change notices). The handbook pages are all the documentation necessary, I believe.

jibran’s picture

Yay!! Great work everybody. So happy to see it in the core.

Here is a latest example of change notice New configuration translation user interface module added.

chx’s picture

Also

  1. The iterator documentation is work ahead of this patch. This patch didn't contain the Iterator. The next one will. When that comes, Map will be documented. And machine_name too cos that'll be in the next one too.
  2. As for repeating information, that's unfortunate but it is not inherent in the architecture of migrate, it's an artefact of the variables source which is downright weird. It's (very) unlikely any similar repetitions will occur for migrations not involving variables as a source
dsnopek’s picture

As a contrib module developer, here is what I'd want in a change notice: "In the past we wrote hook_update_7000() functions to facilitate the upgrade from Drupal 6 to Drupal 7, but now for Drupal 8, you have to do XXX." (Where XXX is link to the Migrate API documentation).

geerlingguy’s picture

@dsnopek - Aren't we still doing module-specific updates inside hook_update_N() functions, though? My understanding was that migrations would be more for entities, blocks, types, etc...

[Edit: Hmm, but if the database is fresh, we couldn't do that anyways, so I guess all my existing hook_update_N() implementations for D8 will need to be converted.]

catch’s picture

@dsnopek, @geerlinguy:

For major upgrades, everything will need a migration handler, no hook_update_N(). This does mean that any existing hook_update_N() in modules will need to be re-written.

For 8.x-8.x updates, hook_update_N() will still be used (at least for the time being). However if you make a schema change, you'd be expected to write a source and destination for that schema change ideally as well - so that it's possible to migrate from/to that version of the schema (i.e. 6.x to the new schema).

A change notice along the lines of what @dsnopek says seems OK, however I'm not sure things are ready at all yet for contrib module authors to start writing migrations for their modules - chx is that even worth trying or should we tell people to hold off for now?

yktdan’s picture

So basically, every contrib module needs to write a migrate piece. What happens when module x is replaced by moduley or modulez, both of which do what x did but each also has their own unique other features?

I have a particular nasty case in mind, namely Ubercart on D6 to Commerce on D8 (or maybe Ubercart will also produce a D8 version as they did for D7. Yes there is a migrate from Ubercart D6 or D7 to Commerce D7 as a starting point for the migration, but both Ubercart and Commerce are sort of like Drupal in having a core and their own contrib features which are not handled by the migrate code.

nikhilsukul’s picture

Hi chx,

I have added the Change notice:
https://drupal.org/node/2141805

catch’s picture

What happens when module x is replaced by moduley or modulez, both of which do what x did but each also has their own unique other features?

Module y and module z would both need to write destination classes for data from module x.

The sources for module x would be the same, so could be shared between y and z.

dsnopek’s picture

@nikhilsukul: Thanks! I made some minor changes too. :-)

webchick’s picture

Status:Active» Needs work

Thanks for starting the change notice!

It's missing an important component though, which is the "this is what you used to do in D7, and here's what you now do in Drupal 8." (see https://drupal.org/node/1935708 for an example.) It's fine if we need to further refine the examples as more of Migrate gets in, and it's fine if it's only one example and not a comprehensive guide (that should go under the Migration API handbook), but we definitely need something to throw contrib authors a bone here.

chx’s picture

I have no idea how to write that. It is a world of a difference. Nothing, at all, applies from a D7-style upgrade. You need to change how you think of these matters. I have NFI how to put that in a change notice. I will brainstorm with the team, stay tuned.

geerlingguy’s picture

It is a world of a difference.

That's exactly why we (contrib devs) need something :)

I know how upgrades went pre-migrate-in-core, and I know how migration classes worked in Drupal 6 and 7, but I don't know how to migrate things like variables into configuration items, etc. I think, at this point, even the most basic example (like how to upgrade one variable using the old way using hook_update_7000() in the .install file, and how to migrate one variable into config in the new way) would be a great starting point.

Plus, it would make sure we can iron out any possible major DX WTFs before migrate starts getting layers of polish.

Thanks for all the hard work on this, and know that it's very much appreciated!

catch’s picture

I think a variable conversion would be a great example - the hook_update_N() with update_variables_to_config(), then the YAML file for comparison. This is also something a lot of modules are going to need to do.

Modules that have custom storage that's neither entities nor variables in Drupal 7 are going to need to read up on writing source/destination classes, but that's a more limited set of developers and a link to the general documentation (and the core migrations once we have them) should be plenty.

We're also going to have to document the workflow for overall migrations at some point (i.e. making it clear that when a module schema changes during a minor release, new source/destination classes need to be written to match that schema if it's incompatible with the previous one, as well as hook_update_N() for minor updates), but that probably needs its own issue to thrash out a bit.

chx’s picture

Status:Needs work» Fixed

An example has been added, converting system_update_8009 to a migration.

chx’s picture

Title:Change notice: Migrate in core: patch #1» Migrate in core: patch #1
iantresman’s picture

Why in core?

I don't know how many installs are upgrades, and how many are fresh installs, but people who are smart enough to do an upgrade/migration, are smart enough to download and enable a module to do this, and then disable/remove it, when it is done.

Grayside’s picture

@iantresman, Migrate in Core is the newest Core Initiative. The time for debate is considerably past.

That said, the basic notion is that traditional use of update hooks is bad, that Migrate will be cleaner, and it is in core because it is going to take the place of the core upgrade path. Not everything in the Migrate contrib module will be going into Core, just the essential APIs and mechanisms to upgrade Drupal 6 and Drupal 7 sites to Drupal 8.

Cottser’s picture

Issue tags:-Needs change record

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.