Add comments to remove entity ids in migration [#2818157]

Follow-up to #2748609: [meta] Preserving auto-increment IDs on migration is fragile

Problem/Motivation

Due to #2748609: [meta] Preserving auto-increment IDs on migration is fragile, we can get a quick win by just adding a comment into node, taxonomy, user and d7_file migration templates that suggestions that you might really, really want null out the entity id so you can actually possibly do incremental migrations.

Proposed resolution

Add a comment to all these migration templates saying you might want to remove the entity id.

Remaining tasks

Create a patch to do this.

User interface changes

API changes

Data model changes

Comment	File	Size	Author
#22	interdiff-18-22.txt	1.03 KB	Ada Hernandez
#22	add_comments_to_remove-2818157-22.patch	6.51 KB	Ada Hernandez
#22	8.3.x: PHP 5.5 & MySQL 5.5 21,003 pass
#13	add_comments_to_remove-2818157-13.patch	5.45 KB	Ada Hernandez
#13	8.3.x: PHP 5.5 & MySQL 5.5 20,998 pass
#13	interdiff-10-13.txt	1.68 KB	Ada Hernandez
#10	interdiff-7-10.txt	4.54 KB	Ada Hernandez
#10	add_comments_to_remove-2818157-10.patch	7.13 KB	Ada Hernandez
#10	8.3.x: PHP 5.5 & MySQL 5.5 20,998 pass
#8	interdiff-3-7.txt	8.41 KB	Ada Hernandez
#8	add_comments_to_remove-2818157-7.patch	8.3 KB	Ada Hernandez
#8	8.3.x: PHP 5.5 & MySQL 5.5 20,977 pass
#5	interdiff-3-5.txt	8.41 KB	Ada Hernandez
#5	add_comments_to_remove-2818157-5.patch	8.41 KB	Ada Hernandez
#5	8.3.x: PHP 5.5 & MySQL 5.5 Patch failed to apply
#3	add_comments_to_remove-2818157-3.patch	9.46 KB	Ada Hernandez
#3	8.3.x: PHP 5.5 & MySQL 5.5 20,972 pass
#18	interdiff-13-18.txt	2.29 KB	Ada Hernandez
#18	add_comments_to_remove-2818157-18.patch	5.48 KB	Ada Hernandez
#18	8.3.x: PHP 5.5 & MySQL 5.5 21,000 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

14 October 2016 at 01:42

heddn created an issue. See original summary.

Comment #2

heddn

English

Nicaragua

CreditAttribution: heddn at MTech, LLC commented 14 October 2016 at 01:44

Issue summary:

View changes

Comment #3

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 26 October 2016 at 17:41

Status:

Active

» Needs review

File	Size
add_comments_to_remove-2818157-3.patch	9.46 KB
8.3.x: PHP 5.5 & MySQL 5.5 20,972 pass

I have added a patch with the commentaries in migration templates.

Comment #4

heddn

English

Nicaragua

CreditAttribution: heddn at MTech, LLC commented 26 October 2016 at 17:58

Status:

Needs review

» Needs work

+++ b/core/modules/file/migration_templates/d7_file.yml
@@ -13,6 +13,8 @@ source:
+  # the fid and uid fields to allow incremental migrations.

The comment for UID should be closer to the uid field. Can we move the uid field up higher so it is right next to the fid?

+++ b/core/modules/node/migration_templates/d6_node.yml
@@ -9,6 +9,8 @@ process:
+  # the nid, uid and vid fields to allow incremental migrations.

Same applies here for uid. Can we move it higher so the comment applies more closely?

+++ b/core/modules/node/migration_templates/d6_node_revision.yml
@@ -6,6 +6,8 @@ deriver: Drupal\node\Plugin\migrate\D6NodeDeriver
+  # the nid, uid and vid fields to allow incremental migrations.

And here. Move uid closer.

+++ b/core/modules/node/migration_templates/d6_node_translation.yml
@@ -7,6 +7,8 @@ source:
+  # the nid and uid fields to allow incremental migrations.

And here.

+++ b/core/modules/node/migration_templates/d7_node.yml
@@ -6,6 +6,8 @@ deriver: Drupal\node\Plugin\migrate\D7NodeDeriver
+  # the nid, uid and vid fields to allow incremental migrations.

And here.

+++ b/core/modules/taxonomy/migration_templates/d7_taxonomy_vocabulary.yml
@@ -5,6 +5,8 @@ migration_tags:
   vid: machine_name

I don't think it makes sense to remove this one. The vid isn't a numeric identifier for taxonomy vocabularies.

+++ b/core/modules/user/migration_templates/d6_user_role.yml
@@ -5,6 +5,8 @@ migration_tags:
+  # If you are using this file to build a custom migration consider removing
+  # the id field to allow incremental migrations.

I think this is similar to vocabularies. We don't want to remove it here.

+++ b/core/modules/user/migration_templates/d7_user_role.yml
@@ -5,6 +5,8 @@ migration_tags:
   plugin: d7_user_role
 process:
+  # If you are using this file to build a custom migration consider removing
+  # the id field to allow incremental migrations.
   id:
     -
       plugin: machine_name

I think this is similar to vocabularies. We don't want to remove it here.

Comment #5

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 26 October 2016 at 20:00

File	Size
add_comments_to_remove-2818157-5.patch	8.41 KB
8.3.x: PHP 5.5 & MySQL 5.5 Patch failed to apply
interdiff-3-5.txt	8.41 KB

1 file was hidden/shown/deleted

File	Size
add_comments_to_remove-2818157-3.patch	9.46 KB
8.3.x: PHP 5.5 & MySQL 5.5 20,972 pass

@heddn thanks for your feedback, I have removed the commentaries unnecessary.

Comment #6

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 26 October 2016 at 19:58

Status:

Needs work

» Needs review

Comment #7

26 October 2016 at 20:02

Status:

Needs review

» Needs work

The last submitted patch, 5: add_comments_to_remove-2818157-5.patch, failed testing.

Comment #8

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 26 October 2016 at 21:01

Status:

Needs work

» Needs review

File	Size
add_comments_to_remove-2818157-7.patch	8.3 KB
8.3.x: PHP 5.5 & MySQL 5.5 20,977 pass
interdiff-3-7.txt	8.41 KB

2 files were hidden/shown/deleted

File	Size
add_comments_to_remove-2818157-5.patch	8.41 KB
8.3.x: PHP 5.5 & MySQL 5.5 Patch failed to apply
interdiff-3-5.txt	8.41 KB

@heddn now is the correct patch :).

Comment #9

mikeryan

he/him

English

Murphysboro, IL, USA

CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented 27 October 2016 at 14:59

Status:

Needs review

» Needs work

```
+++ b/core/modules/file/migration_templates/d7_file.yml
@@ -13,7 +13,10 @@ source:
+  # the fid and uid fields to allow incremental migrations.
```
The uid mapping should not be removed. If IDs are not preserved, the uid mapping should use the migration plugin instead of a straight mapping - but we have another issue to do that in general.

+++ b/core/modules/node/migration_templates/d6_node.yml
@@ -9,14 +9,16 @@ process:
+  # the nid and uid fields to allow incremental migrations.

Ditto, uid mapping should not be removed.

+++ b/core/modules/node/migration_templates/d6_node_revision.yml
@@ -6,14 +6,16 @@ deriver: Drupal\node\Plugin\migrate\D6NodeDeriver
+  uid: node_uid

Ditto.

+++ b/core/modules/node/migration_templates/d6_node_translation.yml
@@ -7,14 +7,16 @@ source:
+  # the nid and uid fields to allow incremental migrations.

Ditto.

+++ b/core/modules/node/migration_templates/d7_node.yml
@@ -6,14 +6,16 @@ deriver: Drupal\node\Plugin\migrate\D7NodeDeriver
+  # the nid and uid fields to allow incremental migrations.

Ditto, and etc. for the rest (I need to leave atm, haven't reviewed the rest...).

Comment #10

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 28 October 2016 at 16:10

Status:

Needs work

» Needs review

File	Size
add_comments_to_remove-2818157-10.patch	7.13 KB
8.3.x: PHP 5.5 & MySQL 5.5 20,998 pass
interdiff-7-10.txt	4.54 KB

2 files were hidden/shown/deleted

File	Size
add_comments_to_remove-2818157-7.patch	8.3 KB
8.3.x: PHP 5.5 & MySQL 5.5 20,977 pass
interdiff-3-7.txt	8.41 KB

@mikeryan thanks. I have removed the uid for templates migration now.

Comment #11

mikeryan

he/him

English

Murphysboro, IL, USA

CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented 28 October 2016 at 16:31

Status:

Needs review

» Needs work

So, to be clearer and more complete than my dashed-off review yesterday - we only need to remove ID mappings from a migration when they are the primary ID of the entity being created by that migration. There are a number of subsidiary migrations which do not create entities themselves, where the primary ID is of an existing entity being updated - that ID must be mapped for the right entity to be updated.

With the changes below, I'll be ready to RTBC this. Thanks!

+++ b/core/modules/taxonomy/migration_templates/d6_term_node.yml
@@ -6,6 +6,8 @@ deriver: Drupal\taxonomy\Plugin\migrate\D6TermNodeDeriver
+  # If you are using this file to build a custom migration consider removing
+  # the nid field to allow incremental migrations.

Actually, this one should not be removed, because it is properly mapping to the migrated nid (whether nids were preserved or not).

+++ b/core/modules/user/migration_templates/d6_profile_values.yml
@@ -10,6 +10,8 @@ source:
+  # If you are using this file to build a custom migration consider removing
+  # the uid field to allow incremental migrations.

Similar to my previous uid reference - this must be mapped, and really should be using the migration plugin to work when IDs are not preserved, but we have a separate issue for that.

+++ b/core/modules/user/migration_templates/d6_user_contact_settings.yml
@@ -8,6 +8,8 @@ source:
+  # If you are using this file to build a custom migration consider removing
+  # the uid field to allow incremental migrations.

Ditto.

Comment #12

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 28 October 2016 at 17:32

@mikeryan thank you very much, I'll work in that

Comment #13

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 28 October 2016 at 17:40

Status:

Needs work

» Needs review

File	Size
add_comments_to_remove-2818157-13.patch	5.45 KB
8.3.x: PHP 5.5 & MySQL 5.5 20,998 pass
interdiff-10-13.txt	1.68 KB

2 files were hidden/shown/deleted

File	Size
add_comments_to_remove-2818157-10.patch	7.13 KB
8.3.x: PHP 5.5 & MySQL 5.5 20,998 pass
interdiff-7-10.txt	4.54 KB

I updated the patch I removed the commentary for the ID must be mapped now I think this is correct.

Comment #14

mikeryan

he/him

English

Murphysboro, IL, USA

CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented 28 October 2016 at 20:55

Status:

Needs review

» Reviewed & tested by the community

That does it, thanks!

Comment #15

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 28 October 2016 at 21:17

It is a pleasure to help community

Comment #16

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Chapter Three commented 29 October 2016 at 07:11

Status:

Reviewed & tested by the community

» Needs review

+++ b/core/modules/node/migration_templates/d6_node.yml
@@ -9,6 +9,8 @@ process:
+  # If you are using this file to build a custom migration consider removing
+  # the nid field to allow incremental migrations.
   nid: tnid
   vid: vid

+++ b/core/modules/node/migration_templates/d6_node_revision.yml
@@ -6,6 +6,8 @@ deriver: Drupal\node\Plugin\migrate\D6NodeDeriver
+  # If you are using this file to build a custom migration consider removing
+  # the nid field to allow incremental migrations.
   nid: nid
   vid: vid

+++ b/core/modules/node/migration_templates/d7_node.yml
@@ -6,6 +6,8 @@ deriver: Drupal\node\Plugin\migrate\D7NodeDeriver
+  # If you are using this file to build a custom migration consider removing
+  # the nid field to allow incremental migrations.
   nid: nid
   vid: vid

+++ b/core/modules/node/migration_templates/d7_node_revision.yml
@@ -6,6 +6,8 @@ deriver: Drupal\node\Plugin\migrate\D7NodeDeriver
+  # If you are using this file to build a custom migration consider removing
+  # out the nid field to allow incremental migrations.
   nid: nid
   vid: vid

Wouldn't you also have to comment the vid for the node migrations? (I might be wrong - but I think I had to do this on a recent migration).

Also aren't we missing comments?

Comment #17

mikeryan

he/him

English

Murphysboro, IL, USA

CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented 29 October 2016 at 15:30

Status:

Needs review

» Needs work

Ah, right, should have caught that, thanks!

Comment #18

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 31 October 2016 at 19:52

File	Size
add_comments_to_remove-2818157-18.patch	5.48 KB
8.3.x: PHP 5.5 & MySQL 5.5 21,000 pass
interdiff-13-18.txt	2.29 KB

Hi everyone, it did the changes for node templates

Comment #19

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 31 October 2016 at 19:52

Status:

Needs work

» Needs review

Comment #20

mikeryan

he/him

English

Murphysboro, IL, USA

CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented 31 October 2016 at 19:58

Status:

Needs review

» Needs work

There's also alexpott's second comment:

Also aren't we missing comments?

I.e., we need to also document this for the cid mappings in d6_comment and d7_comment.

Comment #21

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 31 October 2016 at 21:46

Oh!, ok thanks

Comment #22

Ada Hernandez CreditAttribution: Ada Hernandez at MTech, LLC commented 31 October 2016 at 22:35

Status:

Needs work

» Needs review

File	Size
add_comments_to_remove-2818157-22.patch	6.51 KB
8.3.x: PHP 5.5 & MySQL 5.5 21,003 pass
interdiff-18-22.txt	1.03 KB

2 files were hidden/shown/deleted

File	Size
interdiff-10-13.txt	1.68 KB
add_comments_to_remove-2818157-13.patch	5.45 KB
8.3.x: PHP 5.5 & MySQL 5.5 20,998 pass

I have added commentaries for comment migration template

Comment #23

mikeryan

he/him

English

Murphysboro, IL, USA

CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented 1 November 2016 at 14:12

Status:

Needs review

» Reviewed & tested by the community

Thanks!

Comment #24

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Chapter Three commented 3 November 2016 at 15:15

Status:

Reviewed & tested by the community

» Fixed

Committed and pushed f1b66f3 to 8.3.x and d76893e to 8.2.x. Thanks!

Adding to 8.2.x because this is just documentation.

Comment #25

3 November 2016 at 15:15

alexpott committed f1b66f3 on 8.3.x

Issue #2818157 by Adita, mikeryan, heddn, alexpott: Add comments to...

Comment #26

3 November 2016 at 15:15

alexpott committed d76893e on 8.2.x

Issue #2818157 by Adita, mikeryan, heddn, alexpott: Add comments to...

Comment #27

17 November 2016 at 15:15

Status:

Fixed

» Closed (fixed)

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

Add comments to remove entity ids in migration

Problem/Motivation

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Comments

Parent issue

Referenced by