So, migration load plugins are almost a thing of the past! YAY!

However, while working on #2530030: Create the migrate builder plugin type, I neglected (read: forgot) to write a builder for the d6_profile_values migration, which uses a load plugin. Before load plugins can be removed, there needs to be a builder plugin which will take over those duties. I have attached one, with a test.

CommentFileSizeAuthor
#13 interdiff-2549003-11-13.txt670 bytesphenaproxima
#13 2549003-13.patch1.24 KBphenaproxima
#11 interdiff.txt1.08 KBmikeryan
#11 create_a_builder_for-2549003-11.patch1.13 KBmikeryan
#8 2549003-8.patch766 bytesphenaproxima
#3 interdiff-2549003-0-3.txt601 bytesphenaproxima
#3 2549003-3.patch2.96 KBphenaproxima
migrate-profile_values_builder.patch2.96 KBphenaproxima
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phenaproxima created an issue. See original summary.

Status: Needs review » Needs work

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

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs work » Needs review
FileSize
2549003-3.patch2.96 KB
interdiff-2549003-0-3.txt601 bytes

D'oh.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented
Status: Needs review » Reviewed & tested by the community
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Issue tags: +blocker

This is blocking some important stuff.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Priority: Normal » Major
Status: Reviewed & tested by the community » Fixed

This sounded a lot scarier than it actually was. :) Turns out the change is very simple and is really just a follow-up from the builder patch that was accidentally omitted.

I inquired about why we need this in the first place, since the wrapping code here is very trivial. Adam pointed out it's because this essentially acts as an "alter" hook to let us pretend that profile fields are real fields.

Builders and load plugins are pretty much exactly that -- they are plugin-ized alter hooks for migrations.
Builders are kind of like hook_presave, and load plugins are kind of like hook_load.
They're not real CCK or Field API fields, but for the sake of this migration we can pretend they are

Works for me.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed b16b01d on 8.0.x 
    Issue #2549003 by phenaproxima: Create a builder for the...
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Fixed » Needs review
FileSize
2549003-8.patch766 bytes

Whoops! We (read: I, phenaproxima) screwed up a line that broke Migrate Upgrade. This is the fix.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented

Is there any way to add test coverage for that?

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

This patch fails if you do not have profile installed in D6...

SQLSTATE[42S02]: Base table or view not found: 1146 Table 'd6.profile_fields' doesn't exist: SELECT pf.fid AS fid, pf.title AS title, pf.name AS name, pf.explanation AS explanation, pf.category AS category, pf.page AS page, pf.type AS type, pf.weight AS weight, pf.required AS required, pf.register AS register, pf.visibility AS visibility, pf.autocomplete AS autocomplete, pf.options AS options FROM {profile_fields} pf; Array ( )

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented
FileSize
create_a_builder_for-2549003-11.patch1.13 KB
interdiff.txt1.08 KB

We need to check the requirements (specifically source_provider=profile).

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented
Issue tags: +Migrate critical

Migrate critical, since it totally breaks migrate-upgrade.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
FileSize
2549003-13.patch1.24 KB
interdiff-2549003-11-13.txt670 bytes

Added a comment mentioning the need to pass database connections to Migrate SQL sources, rather than letting them create connections out of thin air.

The last submitted patch, 11: create_a_builder_for-2549003-11.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Needs review » Fixed
Related issues: +#2552791: MigrateSqlSource should use dependency injection

Ok, let's get this in. Tests are being deferred to #2552791: MigrateSqlSource should use dependency injection to unblock Migrate Upgrade. It's not straight-forward.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 9f73211 on 8.0.x 
    Issue #2549003 follow-up by phenaproxima: Fix regression in Migrate...

Status: Fixed » Closed (fixed)

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