Migrate now allows a proper way for class registration.

Files: 
CommentFileSizeAuthor
#27 1866950-og-migrate-26.patch2.79 KBamitaibu
PASSED: [[SimpleTest]]: [MySQL] 780 pass(es).
[ View ]
og-migrate2.5.patch5.83 KBamitaibu
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch og-migrate2.5.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

amitaibu’s picture

Status:Needs review» Fixed

Committed.

danylevskyi’s picture

Status:Fixed» Needs work

Amitaibu, your patch causes problem with Drush migrate command.
Please, see this: #1869052: drush mi: Failure to sort migration list

amitaibu’s picture

+++ b/og_ui/og_ui.moduleundefined
@@ -1122,6 +1122,7 @@ function og_ui_migrate_api() {
+    $migrations['OgUiPopulateField'] = array('class_name' => 'OgUiPopulateField');

Maybe that caused your error -- that's the only "big" change in the patch. That migrate was never registered, although it should.

danylevskyi’s picture

I'll investigate the problem later, but I am 100% sure that problem in this commit.

amitaibu’s picture

I believe you :)

Try removing $migrations['OgUiPopulateField'] = array('class_name' => 'OgUiPopulateField'); to see if that's the cause.

btw, what are you working on? :)

danylevskyi’s picture

It seems that problem in "if (db_table_exists('d6_og'))" statements.
Removing $migrations['OgUiPopulateField'] doesn't remove error.

amitaibu’s picture

Are you with Migrate 2.5?

danylevskyi’s picture

Yes, Migrate 2.5

amitaibu’s picture

> It seems that problem in "if (db_table_exists('d6_og'))" statements.

You mean removing them caused the WSOD? Sounds like a Migrate issue, especially since it works ok via the UI.

danylevskyi’s picture

Yes, I mean removing them caused error.
Need more time to investigate...

jpklein’s picture

I started getting a WSOD via the UI at "admin/content/migrate" after upgrading OG from 2.0-beta3 to rc1. Issuing "drush ms" from the command-line displayed the following error message: "Failure to sort migration list - most likely due to circular dependencies involving OgMigrateContent,OgMigrateUser,OgUiMigrateAddField,OgUiSetRoles". I tried to "migrate-deregister" the offending migrate tasks, but to no avail. Reversing this patch was the only solution I found.

Please reverse this patch in the next release candidate, or fix the edge case where someone has already run the og migrations with an earlier beta.

amitaibu’s picture

> Please reverse this patch in the next release candidate, or fix the edge case where someone has already run the og migrations with an earlier beta.

How about a patch to help -- maybe you can write the hok_update_N() to de-register the migrate plugins?

arosboro’s picture

any word on a patch here? I ran migrate using the beta2 of og

danylevskyi’s picture

I had some experiments with this bug today. Adding if statements to class definitions fixes this issue. But! Another very weird issue appears. See this: #1814264: Custom migration classes confused by default og-to-og migration classes: Base table or view not found: 1146 Table 'd6_og'.

danylevskyi’s picture

Amitaibu, my proposal is to move migration classes for OG content updating into separate submodule. This is roadmap:

  1. Create submodule called og_migrate_data and make it dependent on OG and Migrate UI.
  2. Move migration classes for updating data from OG and OG UI to new OG Migrate Data module.
  3. Write hook_update_N for OG and OG UI to deregister unwanted classes.
  4. Update message returning in update hooks in OG and OG UI: Enable "OG Migrate Data" module to continue the migration of data. Once migration is done you may disable "Migrate" module.
  5. Profit!

Amitaibu, I am ready to prepare patch. What do you think about this proposal?

amitaibu’s picture

> Amitaibu, I am ready to prepare patch

Much appreciated!

I'd like to better understans -- Wouldn't 3 be enough, i.e. de-register classes on hook_update_N() so later users can re-register them?
Also I'm still not clear what is the benefit of another module.

danylevskyi’s picture

Also I'm still not clear what is the benefit of another module.

Now we have a problem on fresh install with migration classes for updating content #1869052: drush mi: Failure to sort migration list.
To fix this problem we can add if statements before classes to check if migration is needed. But these statements create another problem #1814264: Custom migration classes confused by default og-to-og migration classes: Base table or view not found: 1146 Table 'd6_og'. We got something like circle :)

Our goal is to include migration classes only if we need to make content migration (while updating from one version to another). Additional submodule will fix this problem. It can be enabled or disabled if needed.

I'd like to better understans -- Wouldn't 3 be enough, i.e. de-register classes on hook_update_N() so later users can re-register them?

Yes, it's enough. After enabling "og_migrate_data" module classes will be registered automatically.

amitaibu’s picture

> Our goal is to include migration classes only if we need to make content migration

But I already have an IF for registration of migrations handlers inside OG / OG-UI
It dosen't do any dis-registration -- is that the problem?

What I'm thinking is that we can de-register all the migrate classes, and let the user re-register them via the Migrate UI/Drush.

amitaibu’s picture

> But these statements create another problem #1814264: Custom migration classes confused by default og-to-og migration classes: Base table or view not found: 1146 Table 'd6_og'. We got something like circle :)

I think the question is if using migrate 2.5 VS 2.4. The 2.4 will register it no matter what. If using the latest recommended version wrong migrates won't be registered.

danylevskyi’s picture

Wow! New information. Problem raises only after calling migrate_autoregister() function. Migrate 2.5.
Try to run drush mar and go to /admin/content/migrate.

amitaibu’s picture

Can you please check -- if you set variable_set('migrate_disable_autoregistration', TRUE) -- and then enable OG I would assume it won't crash, because the handlers won't be registered.

danylevskyi’s picture

Fresh install. Migrate 2.5. After installing OG all works great.
After drush mar OG crashes regardless of migrate_disable_autoregistration variable.

amitaibu’s picture

Project:Organic groups» Migrate
Component:og.module» Code
Category:task» support
Issue tags:+OG 2.x release blocker

@mikeryan , I've Moved the issue to Migrate, as I'm not clear if it's an OG or Migrate bug.
Seems that Migrate is registration classes it should not register. Any chance you can have a look?

mikeryan’s picture

Project:Migrate» Organic groups
Component:Code» og.module
Category:support» task

Sounds like there are multiple problems here - a WSOD and a "circular dependencies" error? The latter can indicate, besides circular dependencies, a class that's not defined, so make sure all the classes being registered in hook_migrate_api() are in fact defined, in .inc files that are referenced in the appropriate .info file. On that score, pulling the current -dev of og, I see og.info references a couple of non-existent files in includes/migrate/7200, og_ogur*. Actually the opposite of what's being reported, but it would be worth auditing to make sure there's a one-to-one correspondence between the .inc files and the files[] listed in .info.

alesr’s picture

I got this "circular dependancies" error today when trying to migrate from OG 7.1 to 7.2. (Migrate 7.x-2.x-dev+30, OG 7.x-2.0-rc2)

After debugging I focused on line 90 in migrate.module where $ready = FALSE is set..

        if (!isset($migrations[$dependency])) {
          $ready = FALSE;
          break;
        }

Commenting $ready = FALSE; solves the WSOD problem. Still get those errors on admin/content/migrate page but maybe it can help us out solving the problem...

SQLSTATE[42S02]: Base table or view not found: 1146 Table 'intaview_new.d6_og_ancestry' doesn't exist
SQLSTATE[42S02]: Base table or view not found: 1146 Table 'intaview_new.d6_og_uid' doesn't exist
SQLSTATE[42S02]: Base table or view not found: 1146 Table 'intaview_new.d6_og' doesn't exist
amitaibu’s picture

Status:Needs work» Needs review

Thanks @mikeryan for going over this!

I've deleted the obsolete classes from the info file.
In this patch I've moved the register into the hook_migrate_api() -- @danylevskyi can you test is please?

amitaibu’s picture

StatusFileSize
new2.79 KB
PASSED: [[SimpleTest]]: [MySQL] 780 pass(es).
[ View ]

And the patch :)

amitaibu’s picture

Status:Needs review» Fixed

I've committed the patch, as Migrate test passed locally. Please re-open if still not solved, or better -- confirm with comment it's ok, as this is a release blocker.

torrance123’s picture

Priority:Normal» Critical
Status:Fixed» Needs work

I'm using rc3 and have confirmed that the patch in #27 is included in this release (as attempting to apply the patch fails with "Reversed (or previously applied) patch detected!").

I'm still getting the following error when I attempt to access either /admin/content/migrate or when attempting to manually register migrations:

MigrateException: Failure to sort migration list - most likely due to circular dependencies involving OgMigrateContent,OgMigrateUser,OgUiMigrateAddField,OgUiSetRoles in migrate_migrations() (line 80 of sites/all/modules/migrate/migrate.module).

Setting back to open and to critical since this is a release blocker.

Edit: to be clear, I'm encountering this issue whilst trying to upgrade from 1.x to 2.x following the instructions here.

danylevskyi’s picture

On fresh install after drush mar Migrate/OG still crashes.

amitaibu’s picture

> or when attempting to manually register

> On fresh install after drush mar Migrate/OG still crashes.

using drush mar or manual registration is wrong -- as it registers handlers that are not defined in hook_migrate_api(), thus overrides al out IF logic in og_migrate_api(). IMO, having this button is wrong and probably meant for backward compatibility

torrance123’s picture

using drush mar or manual registration is wrong -- as it registers handlers that are not defined in hook_migrate_api(), thus overrides al out IF logic in og_migrate_api(). IMO, having this button is wrong and probably meant for backward compatibility

OK, so the instructions here which specify to perform a manual registration are clearly wrong then.

I was able to resolve this by going to the db table migrate_status and deleting all the og migration records from here. I then cleared the cache and revisited admin/content/migrate. This then inserted only 3 migrations into the migrate_status table (previously there had been 6) and avoided any conflicts.

My migrations are running now.

amitaibu’s picture

Cool - maybe you can fix the doc and improve it for others? :)

amitaibu’s picture

Status:Needs work» Fixed

I've updated the docs myself, so closing.

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