Problem/Motivation

The UI for upgrades to Drupal 8 from Drupal 6/7 is being developed in contrib, to be pulled into core when "ready" (the current hope is to get it in for RC1). This UI is under development as migrate_upgrade.

The detailed current design plan is in a Google doc - anyone can read and comment there. Broadly, the workflow should be as follows:

Initial upgrade

  1. Overview page (at http://example.com/upgrade), describing what is to happen and how to prepare for it.
  2. Credentials page - get the DB info for the source site, as well as the address from which to fetch the files.
  3. Report/confirmation page - After analyzing the source database, report on specifically what is going to happen - what source modules will be imported into what destination modules, and what source modules are not being upgraded.
  4. Batch - the migrations are run through the normal Batch API.
  5. Post-upgrade report - message summarizing successful/failed migrations, with link to watchdog listing of specific messages.

Subsequent upgrade

When returning to /upgrade, the following workflow will happen:

  1. Overview page - reports that an upgrade has already been performed, and offers the option of rolling back and starting over, or running an incremental upgrade (importing new content, and content that now has an upgrade path which previously didn't).
  2. If rolling back was chosen, imported configuration entities (e.g., fields, node types, etc.) and content are deleted and migration configuration removed (simple configuration is *not* restored to its original state), and we return to the original overview page.
  3. Otherwise, we skip the credentials page (we already have that information) and go directly to the report/confirmation page, proceeding normally from there.
Files: 
CommentFileSizeAuthor
#79 composer-interdiff.txt479 bytesxjm
#79 migrate-2281691-79.patch63.43 KBxjm
#75 migrate-2281691-76.patch62.97 KBxjm
#74 prescriptive-hook-help.txt1.03 KBxjm
#70 2281691-70.patch62.9 KBGábor Hojtsy
#69 Drupal Upgrade | Hojtsy.hu 2016-01-25 12-09-23.png273.78 KBGábor Hojtsy
#69 Are you sure? | Hojtsy.hu 2016-01-25 12-07-42.png46.41 KBGábor Hojtsy
#69 Drupal Upgrade | Hojtsy.hu 2016-01-25 12-04-30.png105.46 KBGábor Hojtsy
#67 corify.txt1.05 KBmikeryan
#67 meta_user_interface-2281691-67.patch60.96 KBmikeryan
#60 pre-upgrade-report.png32.55 KByoroy
#58 upgrade-flow-1.png40.27 KByoroy
#57 Screen Shot 2015-09-16 at 2.39.06 PM.png142.02 KBwebchick
#56 Screen Shot 2015-09-16 at 2.30.59 PM.png99.97 KBwebchick
#56 Screen Shot 2015-09-16 at 2.29.26 PM.png57 KBwebchick
#56 Screen Shot 2015-09-16 at 2.28.36 PM.png120.52 KBwebchick
#56 Screen Shot 2015-09-16 at 2.26.40 PM.png41.4 KBwebchick
#56 Screen Shot 2015-09-16 at 2.23.38 PM.png13.05 KBwebchick
#56 Screen Shot 2015-09-16 at 2.19.22 PM.png48.21 KBwebchick
#56 Screen Shot 2015-09-16 at 2.16.28 PM.png83.47 KBwebchick
#56 Screen Shot 2015-09-16 at 2.12.41 PM.png96.62 KBwebchick
#53 migration_dashboard.jpg122.19 KBmikeryan
#53 meta_user_interface-2281691-53.patch45.74 KBmikeryan
#32 meta_user_interface-2281691-32.patch23.54 KBmikeryan
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 107,595 pass(es). View
#10 migrate_upgrade_import_complete.jpg125.29 KBmikeryan
#10 migrate_upgrade_import_content.jpg44.83 KBmikeryan
#10 migrate_upgrade_step3.jpg77.86 KBmikeryan
#10 migrate_upgrade_import_config.jpg46.76 KBmikeryan
#10 migrate_upgrade_step2.jpg62.55 KBmikeryan
#10 migrate_upgrade_step1_bottom.jpg90.37 KBmikeryan
#10 migrate_upgrade_step1_top.jpg103.33 KBmikeryan

Comments

mikeryan’s picture

mikeryan’s picture

Issue summary:View changes
mikeryan’s picture

Issue summary:View changes
mikeryan’s picture

Issue summary:View changes
mikeryan’s picture

Issue summary:View changes
mikeryan’s picture

Issue summary:View changes
mikeryan’s picture

Issue summary:View changes
mikeryan’s picture

Issue summary:View changes
mikeryan’s picture

Issue summary:View changes
mikeryan’s picture

For context, attached are screenshots of the interface as reviewed with Bojhan and webchick - all the current child issues are relative to this version of the UI.

mikeryan’s picture

Issue summary:View changes
mikeryan’s picture

Issue summary:View changes

That's it, all the known child issues at this time have been added.

mikeryan’s picture

So, out of all these child issues, the biggest one is #2282021: Consolidate upgrade process to one form/one click. I'm thinking of simplifying it even further, narrowing the scope of the migrate_upgrade functionality to just a one-click full configuration-and-content import (or maybe allowing a configuration import and subsequent content import). Please weigh in at #2282021: Consolidate upgrade process to one form/one click.

Thanks.

benjy’s picture

Would be nice to have an update on this issue of the current state of the UI, big outstanding issues etc.

mikeryan’s picture

Issue summary:View changes

Added context issue.

mikeryan’s picture

Sorry I missed that question - basically, the UI has been working to the extent the underlying framework has for several months, with occasional chasing-HEAD breakage. I haven't tried it for a few weeks so it might well be broken at the moment, but catching up with recent core changes should take care of that. The open issues aren't really major - either cleanup/improving reuse, or providing more/better feedback - the biggest thing I think is the context issue. Should the migration readiness assessment being discussed be integrated with this upgrade form, or separate? Should it be in contrib - in migrate_plus, or perhaps coder_upgrade?

benjy’s picture

Also, after talking with webchick I thought i'd do an initial review, i forked migrate_upgrade into a sandbox and i've done some general clean-up here: https://www.drupal.org/sandbox/benjy/2448261

@mikeryan, not sure if you want me to post it into one big patch in the migrate upgrade issue queue or if you want me to break it up. The diff is here: http://privatepaste.com/458f93e589

Also, I can confirm, all is working with HEAD as of the last day or two.

mikeryan’s picture

I would like to see it broken up, I see a few different things going on there it'd be easier to review separately:

  1. Database driver setup refactoring.
  2. Version detection changes.
  3. Batch results logging.
  4. Miscellaneous rearrangement and simple coding standard stuff (e.g., use of $this->t()).

On a quick once-over I have some comments/questions, but it'll be easier to discuss in more specific issues.

Thanks!

benjy’s picture

Sorry, I should have mentioned there is an overview on the sandbox overview page.

iantresman’s picture

Just to add my support for a user interface (UI). Most people running Drupal sites are not programmer/developers, and without a UI, will find migration very difficult, if not impossible.

I envisage an option to specify a Drupal 6/7 mySql hostname, username and password, and the migration system to investigate what may be migrated, followed by an option to select data to migrate.

I believe the node import module provides a UI to import/migrate from CSV files.

webchick’s picture

Priority:Normal» Major
Issue tags:+Migrate critical

Escalating to major, and also creating the "Migrate critical" tag since this is actually critical (but not critical enough to block D8's release).

#2030501: [meta] Ensure that Drupal 6 sites have a functional upgrade path to either Drupal 7 or 8 before Drupal 6 loses security support is a D8 critical tracking this.

mikeryan’s picture

An update - right now, the main thing I want to get into the UI under development as migrate_upgrade before submitting to core is #2463909: Migrations should support non-installed default configurations (templates) - been busy pursuing other basic core API issues, but I'll try to get my focus back on that one. Actually, the next thing I plan to do is #2504759: Implement drush migrate-upgrade command, which will make it easier to develop/test the template support.

webchick’s picture

Status:Active» Needs review
Issue tags:+Needs usability review, +needs accessibility review

So while there's no patch here yet, https://www.drupal.org/project/migrate_upgrade actually exists and is functional. (I tried it myself last week on both mine and Dries's blogs :)).

If we want to get this in before RC1 (which really only makes sense once the other migrate criticals are resolved), we need to do some reviews of it ASAP from a UX/a11y POV. Tagging.

webchick’s picture

Testing instructions + screenshot at https://www.drupal.org/node/2257723

mikeryan’s picture

Quick question - what should the module name be for the UI? The existing contrib migrate_upgrade module contains both UI and a drush command, and at least for the short term (until we follow through with submitting it to the drush project) continue to exist for the drush. Should the upgrade UI in core be migrate_upgrade_ui? Or perhaps migrate_drupal_ui?

andypost’s picture

is this UI depends on migrate_drupal or migrate module itself?

mikeryan’s picture

Yes - it's a UI to run the migrations defined in migrate_drupal.

Pending any objections, right now I'm doing it as migrate_drupal_ui...

mikeryan’s picture

mikeryan’s picture

Oh, on the migrate_drupal_ui name - I just want to point out a counter-argument to using that, which is that while migrate_drupal provides a general framework for Drupal-to-Drupal migrations, this UI is narrowly focused on the straight, complete upgrade use case - a general-purpose UI for doing custom Drupal-to-Drupal migrations will go into contrib (D8 version of migrate_d2d).

webchick’s picture

I would just stick it in the Migrate Drupal module directly, personally. That might not be popular. :)

benjy’s picture

I would just stick it in the Migrate Drupal module directly, personally. That might not be popular. :)

I don't mind that idea if we could wrap it in a permission.

Otherwise migrate_upgrade_ui is probably better for contrib and migrate_drupal_ui better for cores more specific use case.

mikeryan’s picture

Issue tags:+Needs tests
StatusFileSize
new23.54 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 107,595 pass(es). View

Here's a patch adding the upgrade UI to core as module migrate_drupal_ui. Obviously needs tests, UX work, etc., but it is functional. I've done some cleanup relative to the contrib migrate_upgrade version - the most visible change is providing more feedback while running the migrations.

Heading off the first question a code reviewer will have looking at this patch in isolation - what's with this MigrationCreationTrait and why isn't it simply implemented within MigrateUpgradeForm? The reason is the exact same functionality is needed by the migrate-upgrade drush command (currently in the contrib migrate_upgrade module, ultimately to be added to drush itself) - rather than having the drush command (or, for that matter, any alternate upgrade front ends people might want to build) reinvent the wheel, it should be reusable. We could make it a class instead - maybe even a service - feedback welcome.

andypost’s picture

  1. +++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.info.yml
    @@ -0,0 +1,10 @@
    +dependencies:
    +  - migrate
    +  - migrate_drupal

    +++ b/core/modules/migrate_drupal_ui/src/Controller/MigrateController.php
    @@ -0,0 +1,24 @@
    +    return $this->redirect('dblog.overview');

    so it depends on migrate_drupal and dblog

  2. +++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.install
    @@ -0,0 +1,16 @@
    +function migrate_drupal_ui_install() {
    +  $url = Url::fromUri('base:upgrade')->toString();
    +  drupal_set_message(t("The Migrate Drupal UI module has been enabled. Proceed to the <a href='@url'>upgrade form</a>.", array('@url' => $url)));

    useless

  3. +++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.module
    @@ -0,0 +1,19 @@
    + * @file
    + * Alert administrators before starting the import process.

    module do different

  4. +++ b/core/modules/migrate_drupal_ui/src/Form/MigrateUpgradeForm.php
    @@ -0,0 +1,164 @@
    +/*
    +    $form['files'] = array(
    +      '#type' => 'details',

    tbd

mikeryan’s picture

+++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.info.yml
@@ -0,0 +1,10 @@
+package: Core

Just a note that per https://www.drupal.org/node/2313651#comment-10269591, on the next roll of this patch let's set the package to Experimental.

phenaproxima’s picture

I think the Migrate Upgrade module should be rolled straight into Migrate Drupal, not as a submodule. It has no other use beyond Migrate Drupal. It was always intended to work with Migrate Drupal. IMO there's no point hiding the UI while exposing Migrate Drupal's API (such as it is); hide it all or don't.

mikeryan’s picture

Well, the migrate_drupal framework will be useful in contrib (migrate_d2d) to support custom Drupal-to-Drupal migration paths. That being said, I'm imagining migrate_d2d may end up using the core upgrade form anyway (altering it to be the first step of a wizard instead of a standalone form), so having the upgrade UI automatically present when migrate_drupal is enabled isn't necessarily unnecessary...

webchick’s picture

Part #1/N of my review. :)

Ok testing this patch, along with #2532534: Migration Files for Drupal 7 Comments against the database from webchick.net. This is a fairly bare-bones D7 site with no additional contribs other than Omega for the theme and Mollom for the spam fighting. It has been upgraded since Drupal 5, I believe. It also has some cruft (one time I tested advpoll module out, installed wp once or twice), but since most real-world databases will also have cruft, I think that's ok. ;)


First, I got very confused because I couldn’t find the module to turn it on. ;) That’s because it doesn’t yet have the “Core (Experimental)” package like the other Migration modules do.

RECOMMENDATION Change package to “Core (Experimental)”

Once that’s done, it looks like this:

Migrate, Migrate Drupal, and Migrate Drupal UI modules all listed under Core (Experimental) package on modules page.

Turning on Migrate Drupal UI automatically enables the Migrate and Migrate Drupal modules. So far so good!

Upon enabling it you get the following message at the top of the modules page, directing you to the upgrade form at /upgrade.

The Migrate Drupal UI module has been enabled. Please proceed to the upgrade form.

The only problem with that is if you missed that message in that moment, or if someone else enabled this module before you got there, there’s no way to figure out what you’re supposed to do. There’s no help page (link just says “No help is available for module Migrate Drupal UI.” ), and there’s also no “configure” link on the module from the modules page to jump me right there.

COMMIT BLOCKER Need a hook_help(), like all Drupal modules. Part of the docs gate.
RECOMMENDATION Add a “configure” property to the .info file that points off to /upgrade.

Once you meander over to /upgrade, you see this:

Form with fields for source DB and files directory location.

First, it asks for your source DB credentials, then the location of your public files directory.

OBSERVATION Oh sweet, you can enter the location of a local file directory, too. That probably makes a lot more sense than http://, given that would both a) greatly slow down the migration with 100s or 10000s of HTTP requests and b) pummel your live site, if that’s the address you gave, which seems to be what it’s asking for.

RECOMMENDATION I think remove the default here, and switch the help text around to mention the local files directory option first.

OBSERVATION What about private files? Mike pointed out that for D6 there was no distinction between private/public files, so one file destination will get both. D7 though has separate directories, and for that there’s #2547125: D7 file migration should allow selecting public/private/all files. Tagged that one as a Migrate D7 critical.

RECOMMENDATION Add a version selector to the form (at the top, most likely): Drupal 6 or Drupal 7. If Drupal 7 is selected, toggle visibility of a “Private files” text field. (This selector could also be form_altered() by contrib to provide for example a Drupal 5 selection as well, or used as a “rip-cord” in the event that D7 is not ready when we want to support D6 “officially”; just hard-code the field to D6 and let contrib alter other options in.)

RECOMMENDATION Remove the word “public” from that field’s help text if D6 is selected, since it’ll actually import all files in that case.

RECOMMENDATION More minor, but “Source site files/path” sounds a bit overly technical. I would call this “Source Files” (for the details group, to match “Source Database”) and “Files directory” for the textbook label.

Going to run through an actual migration next.

yoroy’s picture

Initial thoughts based on webchicks screenshots above:

Since this is a set of modules for a very specific and one-time scenario, that message with the link to the upgrade form should probably be persistent. Similar to how it works when you're in maintenance mode.

I think we need more careful and consistent wording, it's a mix of 'upgrade' and 'migrate' at the moment.

hass’s picture

Add a version selector to the form (at the top, most likely): Drupal 6 or Drupal 7. If Drupal 7 is selected, toggle visibility of a “Private files” text field.

We may better connect to the source database and detect the source version automatically. May add one extra step or use ajax.

webchick’s picture

Here's part #2/N, aka, "What happens when things go wrong." :)


Ok, heeeere we go! Testing a D7 -> D8 migration now.

First, I fill in my values:

Database/file values

Incidentally, if you fill in total garbage here, the error messages are quite obtuse:

Raw SQL error

RECOMMENDATION It would be ideal if we could re-use the same error handling as the installer, which prints much user-friendlier messages.

OBSERVATION There doesn't seem to be any validation around the file path field? Or at least I wasn't able to trigger it despite typing in garbage values for both local filepath and http:// address (that could've been due to "migrate already happened" errors tho, see below... but in any case we should probably validate the form first before we proceed).

This instantiates a Batch API process, like so:

Batch API indicates various things are being imported.

So far, so good.

Then, you hit a problem, such as:

Lots of PHP problems

Example output:

Missing bundle entity, entity type node_type, entity id story. (/Users/webchick/Sites/8.x/core/lib/Drupal/Core/Entity/EntityType.php:793)
Missing bundle entity, entity type comment_type, entity id comment_node_advpoll_binary. (/Users/webchick/Sites/8.x/core/lib/Drupal/Core/Entity/EntityType.php:793)
Missing bundle entity, entity type comment_type, entity id comment_node_advpoll_ranking. (/Users/webchick/Sites/8.x/core/lib/Drupal/Core/Entity/EntityType.php:793)
Missing bundle entity, entity type comment_type, entity id comment_node_story. (/Users/webchick/Sites/8.x/core/lib/Drupal/Core/Entity/EntityType.php:793)
Missing bundle entity, entity type comment_type, entity id comment_node_page. (/Users/webchick/Sites/8.x/core/lib/Drupal/Core/Entity/EntityType.php:793)
Missing bundle entity, entity type node_type, entity id advpoll_binary. (/Users/webchick/Sites/8.x/core/lib/Drupal/Core/Entity/EntityType.php:793)
Missing bundle entity, entity type node_type, entity id advpoll_ranking. (/Users/webchick/Sites/8.x/core/lib/Drupal/Core/Entity/EntityType.php:793)
Missing bundle entity, entity type node_type, entity id story. (/Users/webchick/Sites/8.x/core/lib/Drupal/Core/Entity/EntityType.php:793)
Missing bundle entity, entity type node_type, entity id story. (/Users/webchick/Sites/8.x/core/lib/Drupal/Core/Entity/EntityType.php:793)

RECOMMENDATION One annoying thing off the bat is that I can see there are errors, but in almost all cases they disappear faster than I can copy/paste or screenshot them. (something about php_code, something about files, something about story). The output upon moving to each step gets overrridden. Ideally, the messages would just append together and I could save the entire thing as a log of what happened (this is what update.php used to do).

RECOMMENDATION These things are errors, so should be styled as errors, so they're not missed.

Once the migration is complete, you’re automatically redirected to your home page with a status message on how you did:

Indicates success/fail of migrations

RECOMMENDATION Here we’re using “Import” which is a third synonymous word with Migrate/Upgrade. Agreed with yoroy; let’s definitely get the terminology cleaned up.

The "Review detailed migration log" link takes me to the normal database log, with the messages filtered down to migrate_drupal_ui automatically (nice):

Database log showing migrate errors

COMMIT BLOCKER However, the big problem is that none of the PHP errors I saw from earlier are included here. :( Only the high-level info on what passed/failed. I tried resetting the db log filters, thinking these were possibly “php” errors and thus wouldn’t show up in the filtered view, but nothing there either. Migrate is somehow eating these PHP errors from the log..?

COMMIT BLOCKER When a migration fails, the message is “Import of Drupal 7 comments failed”. Well, thanks a lot. ;) How did it fail, and where do I start debugging?

OBSERVATION Some of these seem to report false positives. The row at the top of my report, “Imported Drupal 7 node title label” does not have a corresponding failure row, yet obviously did not work because my site still has no nodes. :)

So then you might think to yourself, "Ok, self. Let's apply some more migrate patches and try that migration again!" Oh, except you can't. You get this when you hit the "Perform upgrade" button again:

'Migration' entity with ID 'd7_comment' already exists

COMMIT BLOCKER We need a way to either re-do the migration or to clear it so you can start from scratch again.

RECOMMENDATION That error message is also super obtuse. Can we get that cleaned up to be more human-friendly? :) "Comments were already imported. Clear database and try again." or similar.

webchick’s picture

Two other notes to capture from IRC:

1) The migrate UI is currently associated with "administer site configuration" but on many sites that's given out kind of willy-nilly. We should most likely base it around "Administer software updates" instead. (Generally-speaking, only User 1 should ever be using this.)

2) While untested (I'll test this as soon as I can get something to actually migrate ;)), the likely thing that happens when using this UI is that any existing content/users/etc. you have are going to be overridden with those from the old site. For example, user 1 was admin, now it's "root." Node 1 was "test" now it's "Welcome to our site."

Given that, we probably need to do a couple of things:

- Big, fat, undeniable red warning on this page (maybe even make "Perform upgrade" red instead of "friendly blue") if site has existing content, and what the consequences of that will be.
- Possibly in the installer, make this an explicit choice: "are you installing D8 to build from, or are you installing D8 to migrate to?" (This would be a separate issue, since there's a lot to contend with there.)

webchick’s picture

Just a quick edit to remove display of those screenshots so you can still see the patch.

mikeryan’s picture

Status:Needs review» Needs work

Anything marked "done" here is in the code for the next patch, more complex things will come.

so it depends on migrate_drupal and dblog

Good point - for the moment, I'm adding the explicit dependency to the .info.yml file. But, is requiring dblog a good idea?

RECOMMENDATION Change package to “Core (Experimental)”

Done.

RECOMMENDATION Need a hook_help(), like all Drupal modules. Part of the docs gate.

hook_help() was there, but not for the help.page.migrate_drupal_ui route. Added some initial help text there.

RECOMMENDATION Add a “configure” property to the .info file that points off to /upgrade.

Hmm, it is there and working for me (at least with the changes I've made so far, none of which seemed specifically applicable)?

RECOMMENDATION I think remove the default here, and switch the help text around to mention the local files directory option first.

Done.

RECOMMENDATION Add a version selector to the form (at the top, most likely): Drupal 6 or Drupal 7. If Drupal 7 is selected, toggle visibility of a “Private files” text field. (This selector could also be form_altered() by contrib to provide for example a Drupal 5 selection as well, or used as a “rip-cord” in the event that D7 is not ready when we want to support D6 “officially”; just hard-code the field to D6 and let contrib alter other options in.)

Will do. Note that actually handling private files for D7 depends on #2547125: D7 file migration should allow selecting public/private/all files.

RECOMMENDATION Remove the word “public” from that field’s help text if D6 is selected, since it’ll actually import all files in that case.

Done for the moment, D7 variant to be added as part of the above.

RECOMMENDATION More minor, but “Source site files/path” sounds a bit overly technical. I would call this “Source Files” (for the details group, to match “Source Database”) and “Files directory” for the textbook label.

Done.

Since this is a set of modules for a very specific and one-time scenario, that message with the link to the upgrade form should probably be persistent. Similar to how it works when you're in maintenance mode.

I'll look into how that's done. Related thought - should we require the site to be in maintenance mode before allowing the upgrade operation?

I think we need more careful and consistent wording, it's a mix of 'upgrade' and 'migrate' at the moment.

So, as discussed on IRC - this form is specifically for a direct, simple upgrade from an earlier version of Drupal to Drupal 8, which is a special case of the general concept of migration. Other scenarios (including customized Drupal-to-Drupal migrations) will be supported in contrib and stick to the "migrate" language. So, my original thought was to make sure all user-facing text here uses the "upgrade" language - but, it is built on the migrate and migrate_drupal modules (which support the wider "migrate" use case), and any errors that bubble up from those modules will be using the "migrate" language, which could be confusing. Once we've got the framework nailed down such errors should be rare, but they will happen.

We may better connect to the source database and detect the source version
automatically. May add one extra step or use ajax.

We're doing the automatic detection now. The issue there is that only D7 needs that private file directory specified, and it's not practical to enable this with ajax from the db credentials. We also really want to keep this to a single form if we can. So, an explicit version selection doesn't seem overly burdensome, all things considered - the detection code will still be there to sanity-check the selection.

And one other thing, upgraded array() instances to [].

Now, on to the newer comments...

webchick’s picture

Incidentally, I think we should push the "whether or not to do D6/D7 as an explicit selector and/or how to show the public/private text fields for D7" issue over to #2547125: D7 file migration should allow selecting public/private/all files. That's likely to need some discussion, so we shouldn't hold this UI up which I was able to just successfully use to migrate webchick.net's content over to D8 (with a few migrate bug fix patches). YEAH! :D

mikeryan’s picture

RECOMMENDATION It would be ideal if we could re-use the same error handling as the installer, which prints much user-friendlier messages.

Will look into how the installer nicifies the messages.

OBSERVATION There doesn't seem to be any validation around the file path field? Or at least I wasn't able to trigger it despite typing in garbage values for both local filepath and http:// address (that could've been due to "migrate already happened" errors tho, see below... but in any case we should probably validate the form first before we proceed).

So, file_exists() on a local filepath and... get_headers() on an HTTP URL? Or, anything with ://, no reason you couldn't put the files on ftp://...

RECOMMENDATION One annoying thing off the bat is that I can see there are errors, but in almost all cases they disappear faster than I can copy/paste or screenshot them. (something about php_code, something about files, something about story). The output upon moving to each step gets overrridden. Ideally, the messages would just append together and I could save the entire thing as a log of what happened (this is what update.php used to do).

The problem here in circumstances like this is the volume - the modern batch API puts the messages above the progress bar, which will get shoved below the fold in no time. I'm sure there's a way to move the progress bar above though, but I don't like the idea of unbounded messages being saved and displayed, it can get crazy if some module has an "Undefined index" notice triggered by every node you migrate (all too common in my D7 experience).

RECOMMENDATION These things are errors, so should be styled as errors, so they're not missed.

Will look at that, MigrateMessageCapture will need to get the message levels.

COMMIT BLOCKER However, the big problem is that none of the PHP errors I saw from earlier are included here. :( Only the high-level info on what passed/failed. I tried resetting the db log filters, thinking these were possibly “php” errors and thus wouldn’t show up in the filtered view, but nothing there either. Migrate is somehow eating these PHP errors from the log..?

They're probably in the infamous message tables, we need a way to expose them here in a structured way... or just pipe them to dblog.

COMMIT BLOCKER When a migration fails, the message is “Import of Drupal 7 comments failed”. Well, thanks a lot. ;) How did it fail, and where do I start debugging?

Good question. The reason it failed should have been emitted right before that message (almost certainly because a migration it was dependent on failed). Not sure why you didn't see "Migration d7_comment did not meet the requirements: [details here]" as the log message immediately before that one.

OBSERVATION Some of these seem to report false positives. The row at the top of my report, “Imported Drupal 7 node title label” does not have a corresponding failure row, yet obviously did not work because my site still has no nodes. :)

"Drupal 7 node title label" did most likely succeed - that's (if I'm not mistaken) is just about setting the title label on node types, it doesn't migrate the nodes themselves.

Anyway, clearly messaging in general needs attention - what should go on the batch page, what should go into the dblog, how to expose the further detail from migrate's message tables....

Bojhan’s picture

Mike one way to work on the messaging a bit more is to have them listed here, perhaps as a screenshot with the quoted text. Its quite hard to try and trigger all the situations in which messages could appear to help you in writing better messages.

I notice some of the spacing in webchicks screenshots are off, is this simply artefact of the screenshot or does it not vertically align?

mikeryan’s picture

Mike one way to work on the messaging a bit more is to have them listed here, perhaps as a screenshot with the quoted text. Its quite hard to try and trigger all the situations in which messages could appear to help you in writing better messages.

Some of the kinds of messages seen in Angie's examples are:

  • Simple progress messages. On the batch page as the migration is running you see things like "Imported Drupal 7 field configuration" and "Currently importing Drupal 7 filter format configuration". In the dblog (displayed at the end) you see "Importing Drupal 7 node title label" (this seems kind of pointless) and "Imported Drupal 7 node title label".
  • If a migration as a whole fails, something like "Import of Drupal 7 menu links failed" will appear as the process is running, and in dblog at the end.
  • The SQLSTATE "Unknown database" error is of course being thrown by the Database API, apparently install.php intercepts such errors and makes them look better.
  • "Missing bundle entity, entity type node_type, entity id story. (/Users/webchick/Sites/8.x/core/lib/Drupal/Core/Entity/EntityType.php:793)" is being thrown by the entity system. Since migration touches all sorts of APIs, pretty much any error that entity/configuration/field/etc. systems can generate can be exposed during migration.
  • Not seen here - if a migration depends on another migration which did not complete, there should be a message about that like
    <?php
            $this->t('Migration @id did not meet the requirements. @message @requirements', array(
              '@id' => $this->migration->id(),
              '@message' => $e->getMessage(),
              '@requirements' => $e->getRequirementsString(),
            )), 'error');
    ?>

    I don't know why that didn't show up on the batch page or in the log. I do know the message itself needs work.

I notice some of the spacing in webchicks screenshots are off, is this simply artefact of the screenshot or does it not vertically align?

Do you have a specific example? They look like I would expect.

Going back to bits I missed from Angie:

COMMIT BLOCKER We need a way to either re-do the migration or to clear it so you can start from scratch again.

RECOMMENDATION That error message is also super obtuse. Can we get that cleaned up to be more human-friendly? :) "Comments were already imported. Clear database and try again." or similar.

This form actually is performing two steps - it generates the appropriate migrations based on the source site and destination site configurations (e.g., if the book module isn't enabled on both sites, no book migration config entity will be created), and then it invokes the import operation on those generated migrations. So, we could theoretically offer to delete the existing migrations first, or to overwrite them, but what happens with any partial content/configuration already imported? Content could be rolled back, but no one's keeping track of the original configuration. At any rate, in what circumstances would the results of running it again differ? In this case, with the application of patches, but I don't think recovering from running with bleeding-edge patches is a use-case we need to cover. Indeed, I would argue that this is a backup-your-database-and-be-prepared-to-restore-it operation - it is a major version upgrade, after all.

The migrate UI is currently associated with "administer site configuration" but on many sites that's given out kind of willy-nilly. We should most likely base it around "Administer software updates" instead. (Generally-speaking, only User 1 should ever be using this.)

Done.

While untested (I'll test this as soon as I can get something to actually migrate ;)), the likely thing that happens when using this UI is that any existing content/users/etc. you have are going to be overridden with those from the old site. For example, user 1 was admin, now it's "root." Node 1 was "test" now it's "Welcome to our site."

Well, at the moment user 1 won't get touched, but we need to figure out how to handle it: #2560637: Improve handling of uid 1 during migration

Big, fat, undeniable red warning on this page (maybe even make "Perform upgrade" red instead of "friendly blue") if site has existing content, and what the consequences of that will be.

Backup all your shit! Now!

(dang, style="color: red" doesn't work)

Possibly in the installer, make this an explicit choice: "are you installing D8 to build from, or are you installing D8 to migrate to?" (This would be a separate issue, since there's a lot to contend with there.)

Yes, we discussed this in IRC - if the answer is "upgrading", we would install migrate_drupal_ui (or whatever it's called) and redirect straight to /upgrade at the end of the install process. Here's another consideration, however - if, say, that book module is enabled on the source site but not in D8, book hierarchies will not be migrated. It was an explicit decision early on that the upgrade process would not automatically enable modules in D8, but we do need a space somewhere for the site administrator to have the opportunity to do it. And they need info on what modules are at risk of not coming over, which we can't give them until they've submitted the credentials on the upgrade form...

mikeryan’s picture

So, along with the question of giving administrators information on what stuff won't migrate until the relevant D8 modules are installed, there's https://www.drupal.org/node/2562073#comment-10289183. With the PHP module out of core, we will be converting php_code filters to filter_null, meaning that content using a format with the php_code filter will not display. Big WTF for anyone using php_code who didn't carefully read the release notes.

I know we'd like to have the single form, single click, but I'm feeling more and more we need to have a confirmation page that does some analysis of the source and destination sites before executing the migration. This page might:

  1. List all source site modules that will be imported based on the current configuration.
  2. List any D8 modules not enabled for which there is source data to import (and allow them to be enabled directly from this page?).
  3. If the php_code filter is in use, warn that content using it will not display without either installing the contrib php module, or editing it after migration to set a different format and remove any PHP.

Thoughts?

webchick’s picture

Well, something like that sounds absolutely fantastic to me.

So page 1: Just your DB credentials / File locations.

page 2: "Howdy, partner. It looks like you're running Drupal 7, with 46 contributed modules. You should know the following things about your site:

- EVERYTHING YOU HOLD DEAR WILL BE OVERWRITTEN. MAKE SURE YOU HAVE BACKUPS.

- PHP Code blah blah
- Foo, Bar, Baz modules don't have migrations, their stuff won't be moved over.
- something else.

[Proceed!]

mikeryan’s picture

I think the best way to move forward on the various feedback/ideas is to work on them in the migrate_upgrade project, where they can be done in separate issues/patches, and the improvements made immediately available to people doing upgrades today. It should be fairly simple to script turning that module into a core patch (mv the files, do a few renames, rewrite some namespaces), so at appropriate checkpoints I can post an updated patch here.

webchick’s picture

Issue summary:View changes
Status:Needs work» Postponed

Yep, agreed.

mikeryan’s picture

OK, I've backported all the work I've done here (plus a little more message tweaking) to migrate_upgrade). Next steps:

  1. Create individual issues for outstanding topics above, parented to this issue.
  2. Write a script for regenerating this core patch from the current migrate_upgrade module.
  3. Use that script at appropriate checkpoints to update the patch in this issue.
mikeryan’s picture

Issue summary:View changes
StatusFileSize
new45.74 KB
new122.19 KB

Attached is a core patch generated from the current status of migrate_upgrade - following that project is really the best way to keep up with the evolving upgrade UI, but I can easily regenerate the core patch here periodically.

The most interesting issues being pursued at the moment are #2567119: [meta] What to do when upgrade has already been done? and #2562849: [meta] Confirmation page - in particular, see webchick's feedback at https://www.drupal.org/node/2565159#comment-10329499.

tl;dr, the original idea of a single page enter-credentials-and-boom! is off the table now. The steps involved now look like:

  • First attempt at upgrade:
    1. Enter db credentials/file location
    2. Review, showing what's being upgraded (and what's not being upgraded).
    3. Run the upgrade process and report on it. Currently the reporting is just dblog messages - perhaps it should be something more like the current migrate_tools UI (below)?
  • Subsequent returns to the upgrade form:
    1. Show status of migrations like the current migrate_tools UI?
    2. Recognizing that the upgrade has been previously performed, ask whether it should be rolled back and redone from scratch, or an incremental migration should be done.
      • If the former, roll back migrated content (simple configuration would not be restored), remove migration configuration, and regenerate the configuration and rerun from scratch like the first time.
      • If the latter, create applicable migrations that didn't already exist (don't touch existing migration configuration). Run the entire migration process - existing migrations will pick up any new content, new ones will get all content.

So, the general-purpose dashboard we currently have in the migrate_tools submodule of migrate_plus looks like this:

Of particular note is the Messages column, which links to a list of migration-specific messages (e.g., for a file migration that source file IDs 71 and 189 failed to copy over). For the upgrade case, we would have a simplified version of this - dropping the machine name, status, operations, and perhaps last imported columns...

I'm really a backend developer - this all will go much quicker with help from a front-end developer, and especially with designers actively participating in refining this UI.

(edit: moved stuff belonging here that was accidentally put into the issue summary)

mikeryan’s picture

Issue summary:View changes
webchick’s picture

First, copying over my comment from #2565159: Create basic confirmation page for better visibility...

It's important here to take a step back and look at the UI we're replacing. That UI is update.php. Update.php is not like a shining pinnacle of UX design, but it does accomplish the following things:

0) If something is up that's going to cause you problems down the road, it warns you about that right away and stops you from proceeding:

Error about not having access to run the script.

1) It first warns you about what the process you're about to engage in, and how to take proper precautions.

First screen explains the whys and wherefores and recommends backing up.

2) Next, it shows you what it's about to do and gives you a chance to review it before proceeding:

Shows two pending updates.

3) It performs operations in a batch...

Batch API shows 2 items processed

4) If there were problems, it tells you what happened, with something you can go and Google more to figure out...

Update #8005 failed, with error message.

5) You're able to review detailed logs to determine even more about what happened in order to help with said Googling...

In addition to update errors, shows some DrupalKernel errors which may be a factor.

6) Once you (think you) have fixed your problem you're free to attempt the update again. It will remember what things didn't work and allow you to re-try them:

Shows failed 8005 update again.

7) Repeat until you get a green board (see no more error messages) and then you know you are done:

No errors.

webchick’s picture

Next, let's show screenshots of the current UI as of right now, which I reviewed with @LewisNyman and @yoroy today:

The first screen prompts for your source database credentials, and then the location to your files, either from the public-facing website itself or from a local directory:

Two sections for source database / source files.

This screen already got reviewed in quite a bit of detail above, and I believe all feedback has now been addressed (e.g. we use "upgrade" consistently in the text, etc.). The one thing that Lewis pointed out is that the page title (not pictured), "Drupal Upgrade: Source site information" is non-standard from others, and should probably be shortened to just "Drupal Upgrade" (I think even I could make that patch, so NBD :D).

Pressing the button leads you to an overall summary of what's about to happen:

Source module / destination module / description columns

Two things came up in review:

1) That "Are you sure?" warning should be moved to a standard confirmation form pattern when clicking the "Perform upgrade?" button.

2) The blank cells here are "concerning" (via yoroy) and they also mean two different things: a) there's a bug, or b) it's intentional, e.g. Block content type / Block content body field configuration both relate to block_content migration so don't bother repeating the info. For this purpose, re-utilzing the "table grouping" we use on e.g. the Permissions page is probably the better pattern to employ here:

List of permissions grouped by module name

At the bottom of the form you'll see a variety of rows like this:

No upgrade detected

These need to be styled in error/warning (similar to Status report page) because THIS IS WHERE YOU WILL LOSE DATA. And right now they look like "FYI" kind of rows.

When you click the button you get a batch API screen:

Progress bar with messages

Both Lewis and yoroy were perplexed at why the messages are showing above vs. below the bar. If that's a quick fix, would be great cos atm the bar moves around which is unexpected. (It does make trying to take screen shots fun, though. ;))

Once the upgrade is complete, you'll see a message like this:

Green message saying 44 migrations completed.

One thing I noticed is that if PHP errors or something not specific to migrate were encountered, those will not be flagged here, giving the mistaken impression that everything's hunky-dory when it's not.

You can review the log for migrate-specific messages:

DBLog filtered down to migrate_upgrade errors

No comment, that all looks fine.

If you attempt to re-run the migration, this is what you see:

Migrate entity 'd7_block' already exists.

We need to fix that error message so that it's intelligible. Quick fix suggestion: "A site upgrade has already been attempted. Please restore from backup to try again." Longer fix suggestion: Let me re-run it as many times as I need to, like update.php.

webchick’s picture

StatusFileSize
new142.02 KB

Finally, let's talk about the real sticking point (at least for me), which is... how do I know when I am done?

The migration might've been successful, in that it shows no errors, but the key thing is that unless I'm sure that all of the modules on my old site have been accounted for, I'm not really done. I need to know what's missing so that I can go and download extra modules (e.g. php_code filter needs php module in contrib now).

And if there are missed things, unlike with update.php, I have no opportunity to re-run the upgrade again without starting over from scratch. That by itself is very similar to major version upgrades in update.php, so not a huge deal. However. The two of them together mean you can end up totally hosed. If you mistakenly believe you are done, because it shows you some lovely green status message at the end, and then you launch your new D8 site and people start adding comments, nodes, etc. to it, and then 3 weeks later you're like "oh crap I forgot about the Donkey module" your only recourse is to restore from a 3 week old backup (which means data loss) or start futzing around manually with the DB tables (which means hiring Mike Ryan ;)).

And this "when am I done?" question is really, really tricky to figure out programmatically. Because all Migrate Upgrade has to go on is the source database, aka the system table in D6/7. Here's an excerpt from mine: (viva PHPMyAdmin)

Excerpt from system table

The system table can tell you some useful things, like the machine names of all modules that were on your old site, whether they were enabled or not, and the schema version. Depending on what this table says about you, you might or might not be hosed when you click that "Perform upgrade" button:

  1. If a module was on the old site and has a destination migration on the new site, we don't need to care. Done.
  2. If a module's status is 0 (disabled) and its schema version is -1, it was never installed and we do not care about it at all (contact module). Done.
  3. If a module's status is 0 and its schema version is not -1, then you have a disabled module that may or may not define data and that is horrible. Recommend skipping these too and logging an error.
  4. If a module's status is 1 that means it was enabled in the old database and we need to care about it, or else we risk losing data.
  5. If an enabled module has a schema version > 0 (like 7009 in the case of comment module), that almost guarantees that we will lose data if we cannot find a corresponding destination module. Those should be flagged as a red error with blink tag and capital letters. ;)
  6. If an enabled module has a schema version of 0, we have no effing idea what that means. It could mean that the module defines no data and we can chirp along happily without a migration path (contextual module). Or it might mean that the module defines a table schema and/or variables and just has never needed a hook_update_N(). We have no idea, so we should flag these as warnings, at least.

That last point is particularly bad, because the only way to figure out if it's "NBD" or "OMG data loss" is to:

  • Go into the old module directory and look for a something.install file.
  • If present, check for hook_schema(). If it's there, you're definitely going to lose data.
  • Also grep anywhere in the module at all for a variable_get() call. If present, and the module is the one controlling that data, you're also going to lose data.
  • Probably other things I am not even thinking about.

And this is completely unreasonable to expect the target audience of this UI to do. (Heck, I'm not sure even I could do that as a Drupalist with 10 years of experience and with PHP/SQL chops.)

I don't know a solution to how to tell if you are "done" beyond something like logging all of the tables in the source database, logging all of the tables that migrations in the new site know about (this would have to be a YAML property so it could be checked prior to modules being enabled) and then showing this information in a Debug window or something.

But I definitely foresee lots of people really, really hosing themselves if we ship with a UI that doesn't know how to answer this question. :(

yoroy’s picture

Issue summary:View changes
StatusFileSize
new40.27 KB

This is my understanding of the upgrade process:

screenflow for performing an upgrade

  1. Database credentials, files path etc.
  2. A pre-upgrade report on upgrades paths found and *not* found
  3. A confirmation screen: are you sure you want to start the upgrade
  4. The upgrade-in-progress screen
  5. A report on what happened + option to start over again

Webchick and I discussed mostly what we should be doing at step 2, see #57 for the different scenarios.

webchick’s picture

Ok, after discussing in IRC with @mikeryan and @yoroy...

1) Forget about advanced debugging (e.g. trying to divine whether a missing destination is a problem or not from the system table, listing table names that are unmigrated, etc.) As enumerated in #57 this is a road to pain and tears and bees.
2) Similarly, forget about warning vs. error distinction for missing destinations. A destination for a source module is missing? It's an error.
3) Allow re-running the UI as many times as you wish.
4) Consult the map tables and only migrate stuff not already accounted for, displaying the list of remaining tasks (like update.php does). This way, if you see something you don't expect you have an opportunity to abort before your data is overridden by your old site's info.

This seems reasonable to me and gets rid of the requirement to know if you're done. I still worry people won't be vigilant enough about reviewing what's about to be migrated in subsequent migrations but same problem exists for CMI.

yoroy’s picture

StatusFileSize
new32.55 KB

Updated flow:

- Add an initial screen explaining "lets do this, here's how to prepare", similar to screenshot 1 in #55
- An extra confirmation page is maybe not so necessary after all. Especially if you can safely run the upgrade process multiple times.

For the pre-upgrade report, maybe what is now in the 3rd column (data to be upgraded) could be shown in the pattern used on the Extend page:

We'll have to think more about what the post upgrade screen could look like. Also, running multiple times means keeping track of what was already upgraded and what not. Should that be made visible in the UI?

mikeryan’s picture

All right, since I find trying to figure out what the current consensus in a long, complex meta issue like this the equivalent of understanding a PHP class given only a sequence of commit diffs;-), I've tried to pull together the last couple days' worth of discussion into a spec at https://docs.google.com/document/d/1Mf-IsowOnQNVuW7xYeNjvX2t2J-rBw3bff-y.... I'd like to nail that down so it contains only the consensus current design, particularly as a guide to Barcelona sprinters, but right now there are a lot of questions in there best discussed here in this issue.

Of course, discussing a dozen questions at once in a single issue is chaos... So first, right now, my immediate priority is to implement the incremental migration scenario ("Returning to the upgrade page", last major section of the doc) - simply performing an incremental migration if you go through the credentials page again would be simple enough, but we need some feedback to the administrator first, and it's not clear to me exactly what/how to provide that. My thought at the moment is that you would basically dive right into the pre-upgrade review screen, which would now be enhanced with the status of the previously-run upgrades (green for "nothing new here", yellow for "new content will be imported by this one", ? for "here's a new upgrade path you didn't have before"...). Thoughts?

Also, the last couple of days we haven't discussed the rollback option (throw it all out and start over)...

Thanks.

mikeryan’s picture

*crickets*

Anyway, I've tagged a bunch of migrate_upgrade issues for Barcelona. Until/unless I hear otherwise, I'm following what I've written up in the Google doc. The hope is that at Barcelona better front-end developers than I can dig into the the UI details, before then the issues I want to address are:

Bojhan’s picture

Status:Postponed» Active
Bojhan’s picture

I've read through the various issues, it looks solid to me. It seems to touch upon everything discussed here. We might need to discuss our ability to remove the extra confirmation screen, as less steps is probably better.

I am looking forward to having this in core, and iterating over it.

Can we update the issue summary, and get the understanding from the doc in place? It would be nice to be able to direct people to the correct followups

mikeryan’s picture

At Bojhan's suggestion, created a child issue summarizing UI issues tagged for Barcelona (since they're in the contrib migrate_upgrade module, they aren't visible to people searching the tags in the core queue): #2573403: [meta] Outstanding tasks for upgrade UI.

mikeryan’s picture

Issue summary:View changes

Updated issue summary.

mikeryan’s picture

StatusFileSize
new60.96 KB
new1.05 KB

Haven't refreshed the core patch lately, so here it is (along with the script I use to move the contrib migrate_upgrade module into core). Note that, besides the UI work that's been going on intermittently in contrib, with this version the module is being renamed Drupal Upgrade/drupal_upgrade. Or, should it be simply Upgrade?

catch’s picture

Status:Active» Needs work

Updating to show there's a patch.

Gábor Hojtsy’s picture

Here is a video of how this looks like live with a Drupal 7 migration. Since that is not complete yet, this also demonstrates error display:

https://drive.google.com/file/d/0B6xsrc5BVkagMjdrTHZ3R2o0YXM/view?usp=sh...

One can also rerun migrations either additionally to what was already run or after starting over. I did not put that into the video, however if you go to /upgrade, this is what you see.

Picking re-run goes directly back to the available/missing upgrades list. Picking rollback asks for confirmation:

Once confirmed, it rolls things back in a batch (similarly displayed as the migration itself with a rolling list of notes on what is going on). And then ends up on the start screen of the migration ui:

Gábor Hojtsy’s picture

Version:8.0.x-dev» 8.1.x-dev
Status:Needs work» Needs review
StatusFileSize
new62.9 KB

Rolled a new version of the patch with the script in #67, thanks. Moving to 8.1 for testing.

Status:Needs review» Needs work

The last submitted patch, 70: 2281691-70.patch, failed testing.

xjm’s picture

The hook_help() failure is just about having a standard format of help text on the help page; I'll submit a fix for that.

xjm’s picture

xjm’s picture

Status:Needs work» Needs review
StatusFileSize
new892 bytes
new1.03 KB

It turns out an hour at the gym is not long enough for InstallUninstallTest to complete on my laptop, so here's a combined patch to check whether the failure is fixed.

xjm’s picture

StatusFileSize
new62.97 KB

Fail. Actual combined patch.

The last submitted patch, 74: migrate-2281691-74.patch, failed testing.

Status:Needs review» Needs work

The last submitted patch, 75: migrate-2281691-76.patch, failed testing.

Gábor Hojtsy’s picture

Looks like the only other fail now is the missing composer.json entry.

xjm’s picture

Status:Needs work» Needs review
StatusFileSize
new63.43 KB
new479 bytes

Here's a fix for that too. I guess we could either add this to the script or apply manually.

xjm’s picture

Started a first pass code review. Here are a few quick things I think we should probably fix prior to core inclusion. Probably we can create issues for these in the drupal_upgrade queue.

  1. +++ b/core/modules/drupal_upgrade/src/DrushLogMigrateMessage.php
    @@ -0,0 +1,28 @@
    +    drush_log($message, $type);

    So I'd expect to see some dependency for this injected here. Assuming this code only runs if Drush is available?

  2. +++ b/core/modules/drupal_upgrade/src/Form/MigrateUpgradeForm.php
    @@ -0,0 +1,1178 @@
    +   * @todo: Find a mechanism to derive this information from the migrations
    +   *   themselves.
    +   *
    +   * @var array
    +   */
    +  protected $moduleUpgradePaths = [

    I believe there is an existing issue for this; so I think it makes sense to go forward with this still outstanding. Probably should link that issue in the @todo. Would be good to add a little more documentation for this since it's such an important part of the functionality atm.

  3. +++ b/core/modules/drupal_upgrade/src/Form/MigrateUpgradeForm.php
    @@ -0,0 +1,1178 @@
    +      $desc = "The following items will not be upgraded. " .
    +        'For more information see <a href="https://www.drupal.org/upgrade/migrate"> Upgrading from Drupal 6 or 7 to Drupal 8</a>.';
    ...
    +        '#description' => $this->t($desc),
    ...
    +            '#plain_text' => t($destination_module),
    ...
    +        "</li><li>" . t($missing_count . ' missing upgrade paths') . "</li></ul>",

    +++ b/core/modules/drupal_upgrade/src/MigrateUpgradeRunBatch.php
    @@ -0,0 +1,360 @@
    +            $singular_message = 'Upgraded @migration (processed 1 item total)';
    +            $plural_message = 'Upgraded @migration (processed @num_processed items total)';
    ...
    +            $singular_message = 'Rolled back @migration (processed 1 item total)';
    +            $plural_message = 'Rolled back @migration (processed @num_processed items total)';
    ...
    +          $message = \Drupal::translation()->formatPlural(
    +            $context['sandbox']['num_processed'], $singular_message, $plural_message,
    +            ['@migration' => $migration_name, '@num_processed' => $context['sandbox']['num_processed']]);
    ...
    +            $singular_message = 'Continuing with @migration (processed 1 item)';
    +            $plural_message = 'Continuing with @migration (processed @num_processed items)';
    +          $context['sandbox']['messages'][] = \Drupal::translation()->formatPlural(
    +            static::$numProcessed, $singular_message, $plural_message,
    +            ['@migration' => $migration_name, '@num_processed' => static::$numProcessed]);
    ...
    +          $message = 'Currently upgrading @migration (@current of @max total tasks)';
    ...
    +          $message = 'Currently rolling back @migration (@current of @max total tasks)';
    ...
    +        $context['message'] = t($message,
    +          ['@migration' => $migration_name, '@current' => $context['sandbox']['current'],
    +           '@max' => $context['sandbox']['max']]) . "<br />\n" . $context['message'];

    These are an incorrect uses of t(); l.d.o cannot find the string this way to make it translatable, and it also is a potentially unsafe pattern for contrib to copy.

  4. +++ b/core/modules/drupal_upgrade/src/MigrateUpgradeRunBatch.php
    @@ -0,0 +1,360 @@
    +        drupal_set_message(t('Completed @count successfully.', ['@count' => $translation->formatPlural($successes, '1 upgrade task', '@count upgrade tasks')]));
    ...
    +        drupal_set_message(t('Completed @count successfully.', ['@count' => $translation->formatPlural($successes, '1 rollback task', '@count rollback tasks')]));
    ...
    +        drupal_set_message(t('@count failed', ['@count' => $translation->formatPlural($failures, '1 upgrade', '@count upgrades')]), 'error');
    +        drupal_set_message(t('Upgrade process not completed'), 'error');
    ...
    +        drupal_set_message(t('@count failed', ['@count' => $translation->formatPlural($failures, '1 rollback', '@count rollbacks')]), 'error');
    +        drupal_set_message(t('Rollback process not completed'), 'error');

    I think using formatPlural() within t() like this also can be problematic to translate (in some languages the translation of "failed" would be different depending on the number). So I think these also need to be just formatPlural().