Problem/Motivation

To help with creating schemas for migrations over in #2183957: Provide configuration schema for Migration module we need to break migration_dependencies out into separate keys.

Proposed resolution

Previously dependencies look like this where the ": false" indicated it was an optional dependency.

migration_dependencies:
  - d6_filter_format
  - d6_user_role
  - d6_user_picture_entity_display
  - d6_user_picture_entity_form_display
  - d6_user_picture_file: false

And afterwards they will look like this, as suggested by vijaycs85

migration_dependencies:
  required:
    - d6_filter_format
    - d6_user_role
    - d6_user_picture_entity_display
    - d6_user_picture_entity_form_display
  optional:
    - d6_user_picture_file

Remaining tasks

Review patch.

User interface changes

n/a

API changes

The way migrations declare their dependencies has changed. This probably isn't that much of an issue since it is unlikely that many people have written D8 migrations at this point.

Generating the Patch

Attached is a script that can convert our existing yml files into the new format. Simply copy the script into the config/install directory and run it.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

Status: Needs review » Reviewed & tested by the community

Great! This structure is vijaycs85's idea and it's a good idea.

benjy’s picture

Issue summary: View changes
FileSize
21.98 KB
2.62 KB

I removed an un-needed isset statement and updated the comment above to reflect the changes.

The last submitted patch, migrate-dependencies.patch, failed testing.

benjy’s picture

FileSize
21.94 KB
1.29 KB

Looks like I mixed up the dependency_graph and the requirement_graph on one part.

The last submitted patch, 2: 2263453-1.patch, failed testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: 2263453-2.patch, failed testing.

benjy’s picture

FileSize
23.84 KB
1.9 KB

The yaml update script had a bug which didn't catch migrations with optional dependencies only.

benjy’s picture

Status: Needs work » Needs review
chx’s picture

Status: Needs review » Needs work

In MigrationStorage.php please remove the mention of "hard" and "soft" dependencies. Having two terminology is confusing.

In d6_field.yml, "However it also has no requirements so it will always run so a dependency is enough." -- this is now called an optional dependency.

In d6_user_picture_file we need a very important comment on d6_file. Let me try to explain: in d6_upload we depend on d6_file because we use what d6_file saved into the file_managed table. But d6_user_picture_file does not use those so why the dependency? Because d6_file stored records in {files} and with as with every other entity type, we want to keep IDs and the only way to do that is to ensure that out of every migration that saves into {file_managed} it's d6_files that runs first. Every contrib migration that sources from an arbitrary table and uses the destination entity:file will need to add this optional dependency. Perhaps even add this comment into d6_file too.

benjy’s picture

Status: Needs work » Needs review
FileSize
23.98 KB
1.73 KB

Updated the comments as requested.

benjy’s picture

FileSize
25.16 KB
2.73 KB

I reworded the comment in d6_user_picture_file and added the comment to d6_file and the entity:file destination as well just for good measure.

chx’s picture

Status: Needs review » Reviewed & tested by the community

Wonderful!

vijaycs85’s picture

looks good. +1 to RTBC.

chx’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
2.05 KB
25.19 KB

Obsessing a little over this because I am obsessed over the comments in migrate cos I want it to be very sucessful. On the d6_file comment: it's not d6_files but d6_file and the dependency is optional. (I will put this back to RTBC; only CNR for the bot)

chx’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 14: 2263453_13.patch, failed testing.

benjy’s picture

Status: Needs work » Needs review
FileSize
25.28 KB
benjy’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • Commit fcc5f3b on 8.x by webchick:
    Issue #2263453 by chx, benjy: Split migration_dependencies into two keys...

Status: Fixed » Closed (fixed)

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