Problem/Motivation

  • All Migrate criticals requiring a BC break must be fixed
  • There are several other issues tagged as Migrate BC break.
  • @mikeryan proposed earlier during the RC that the Migrate module itself (not migrate_drupal) could be considered API-complete once the highwater issue was resolved, and that any further BC breaks would be limited to module-specific plugins.

Proposed resolution

  • Consider migrate.module beta as of 8.2.3. The expectations for beta experimental modules are:
    • Beta experimental modules are considered API- and feature-complete. Some early adopters may begin using beta experimental modules on development sites, but should be aware that they are not yet fully supported and may contain bugs.
    • Developers can begin using beta modules' APIs, but should be aware that some things may still change to address bugs. Beta modules are subject to the beta allowed changes policy.
    • An upgrade path may be provided from beta versions, but be aware it may contain critical bugs.
  • Provide BC for any future API changes in migrate.module.
  • Continue to treat migrate_drupal and module-specific implementations as alpha-stability code and change them as needed without requiring BC.

Remaining tasks

Needs signoff from Migrate maintainers and framework and release managers.

  • Migrate maintainer - Adam Globus-Hoenich (phenaproxima) - Approved (#22)
  • Migrate maintainer - Ben Doughtery (benjy) - Approved (#20)
  • Migrate maintainer - Michael Anello (ultimike) - N/A (#19)
  • Migrate maintainer - Mike Ryan (mikeryan) - Approved (#15)
  • Framework manager - Alex Pott (alexpott) - Approved(#27)
  • Release manager - Jess (xjm) - Approved (#21)

Comments

xjm created an issue. See original summary.

xjm’s picture

Gábor Hojtsy’s picture

If this lands before #2795845: Add changelog for Drupal 8.2.0 and happens before 8.2.0, it would also need to come with a patch for changelog.

Gábor Hojtsy’s picture

This did not end up in 8.2.0.

xjm’s picture

We can still make the change in 8.2.1 (etc.) as desired since Migrate is experimental. Just marking it stable would be minor only.

Gábor Hojtsy’s picture

Yup, just wanted to note that we don't need to make the change in the changelog anymore.

heddn’s picture

heddn’s picture

heddn’s picture

heddn’s picture

Cleaning up the issue summary. I think we need to also plan for getting approval from "Migrate maintainers and framework and release managers". It isn't too early to start thinking about that, considering we are down to just a handful of blocking issues. There's essentially 4 issues blocking at this point and some of them are very near to commit.

heddn’s picture

re: #2791041: Migrate source plugins should use dependency injection
Discussed with @phenaproxima and removing this as a blocker from beta. While it is a BC break, it should not block beta.

heddn’s picture

heddn’s picture

With some quick triage this morning, I don't think there are any beta blockers left. Next step is to get approvals. With that, moving this to needs review. And switching to a task, so this is no longer a plan but an actionable thing to work on now.

chx’s picture

For one last time: 8.3 is ~six months away. You all have enough time to finish #2543552: Modernize migration source plugins before calling migrate stable. I know alexpott have voted that down but that was during 8.2.0. Please reconsider. But also take into consideration I am no longer coding Drupal core.

mikeryan’s picture

First off, I would like to say I approve of the direction of #2543552: Modernize migration source plugins, and I do believe it's a good long-term improvement to the APIs. However, I feel that it's too disruptive to do in 8.x.

To achieve stability in a system, one needs to make increasingly less destabilizing changes. Since the migrations-as-plugins patch went in, we've been doing this - we've done our best to maintain BC when making API changes, and those BC breakers we couldn't avoid have had narrow scope and as little as possible disruption for people doing real-world migrations today. As of now, there have been no BC breaks committed since 8.2.0 was released, and we are not contemplating any more, so beta stability as of an 8.2 patch release looks reasonable to me. There are some BC breakers I would have liked to get in myself, but I'm willing to defer in the interests of stability.

While the current experimental alpha status for migrate *permits* us to make major changes, it doesn't give us carte blanche to completely ignore the impact on people using the migration system today. Refactoring source plugins would touch almost everyone developing migrations - there are several contrib source plugins out there in the wild, as well as people developing custom source plugins (at the very least, everyone doing migrations from databases has to have their own SqlBase-derived plugins). Introducing major changes to the source plugin architecture at this point would make the transition from 8.2.x to 8.3.x very painful for both contrib module authors and people working on custom migrations. Yes, I'm being selfish here - due to the moving targets of the migration system, my own contrib modules have needed separate releases for 8.0.x, 8.1.x, and 8.2.x - I very much want those 8.2.x branches to cover 8.3.x and up (as I'm sure users of those modules would appreciate as well).

There are also opportunity costs to consider. A patch like this will consume a great deal of time among the migrate maintainers, as well as any other contributors with an interest - repeated iterations, reviews, testing, followups - not to mention the time contributed module authors and custom implementors will need to react to the changes. This is time that takes away from all the other work to be done - completing D6/D7 migration paths (including i18n, which is still a big hole), general bug reports, enhancements (and, for me at least, contrib tool-building). I feel that this other work is a bigger bang-for-the-buck than refactoring source plugins is at this point. We've already come a long way from the D7 migrate module (the D8 migration projects I've worked on have consumed far fewer hours than equivalent D7 projects did) - I think it's time to consolidate and incrementally improve those gains.

mikeryan’s picture

As for the task here - I approve experimental beta status for the migrate module. I briefly hesitated on whether #2748609: [meta] Preserving auto-increment IDs on migration is fragile should block it, but that really goes to migrate_drupal's stability, not migrate's.

mikeryan’s picture

Issue summary: View changes
mikeryan’s picture

Issue summary: View changes
ultimike’s picture

I've been away from the migrate issues for a few months now, I don't think I'm currently qualified to weigh in on this at the current time.

-mike

benjy’s picture

Yeah +1 from me, there are lots of things we could continue to improve and change, like #2543552: Modernize migration source plugins which would make things better but I think people have been waiting long enough now and the truth of the matter is, even though we're experimental many people are already relying on Migrate and providing them some stability makes sense.

xjm’s picture

Issue summary: View changes

+1 from me as well! Updating the summary. I really agree with what @mikeryan has said in #15.

I think we just need feedback from @phenaproxima.

phenaproxima’s picture

Approved.

xjm’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Great, this can be RTBC then since the only remaining signoffs needed are committer signoffs.

xjm’s picture

Issue summary: View changes
phenaproxima’s picture

I'd like to add that my sentiments pretty much echo @mikeryan's. Yes, it would be nice to modernize the source plugins -- along with a lot of other parts of the migration system. There are many things I'd like to see happen in Migrate, and a lot of technical debt I'd like to pay off. But the current system, for better or worse, is what Drupal 8 ended up with. We've done a lot of excellent work, and we need to focus on the upgrade path. Changing the basic architecture to improve DX, as excellent as that could be, would likely become a colossal API disruption and time sink.

catch’s picture

#2543552: Modernize migration source plugins ought to be doable in an 8.x minor release with a bc layer. We have to do that for anything else in core that's stable, and have been for the past year mostly successfully. #2326721: EditorPluginInterface should extend PluginFormInterface is a small recent plugin refactor with bc for example. So I don't think it's a strict binary between migrate being marked stable and that patch being punted to 9.x. The more we get used to this, the less either/or choices there are to make about fixing bugs (i.e. the 6/7-8 migration paths) vs. refactoring.

alexpott’s picture

Issue summary: View changes

As a framework manager I think migrate is ready to be beta. The only issue that is RTBC that will need to be changed if do this is #2543568: Remove the md_entity destination plugin hack. We've already been asking for bc layers in migrate patches already and moving to beta will make this necessary for more changes. If we want to do #2543568: Remove the md_entity destination plugin hack as is then for me it's for the migrate maintainers to make that call.

alexpott’s picture

Issue summary: View changes
benjy’s picture

If we could commit #2543568: Remove the md_entity destination plugin hack relatively soon and still be beta in 8.2 then i'd prefer that but if it means waiting until 8.3 then I think we can do something different in #2543568: Remove the md_entity destination plugin hack that doesn't break BC.

catch’s picture

I think it's OK to do it first, then mark beta in 8.2 afterwards.

mikeryan’s picture

There's no BC break in the current patch at #2543568: Remove the md_entity destination plugin hack - I should retitle and rewrite the IS for the narrow scope.

Gábor Hojtsy’s picture

I had this question this morning. We are about to modify constructor arguments to both Drupal\migrate_drupal\Plugin\migrate\destination\EntityFieldStorageConfig and Drupal\migrate\Plugin\migrate\destination\EntityConfigBase in #2225717: Add config translation support to migrations and implement for Drupal 6 user profile fields to implement missing features around config translation migration support. Would the commit of this patch disallow those kind of changes? Looking at https://www.drupal.org/core/d8-bc-policy says

The constructor for a service object (one in the container), plugin, or controller is considered internal unless otherwise marked, and may change in a minor release. These are objects where developers are not expected to call the constructor directly in the first place.

Which sounds sort of good, and if we change that in minor releases, does that mean that #2225717: Add config translation support to migrations and implement for Drupal 6 user profile fields can only be committed to the upcoming version and not cherry-picked for the current minor release? (EntityConfigBase is a service class that is subclassed several times for all kinds of config entities and migrate_drupal's EntityFieldStorageConfig extends from that too).

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs review
mikeryan’s picture

heddn, quietone, and I discussed this on the weekly migration call, and we feel that the upcoming BC break in #2225717: Add config translation support to migrations and implement for Drupal 6 user profile fields is a blocker to beta status. Well, perhaps we could do this when we're in beta status:

Developers can begin using beta modules' APIs, but should be aware that some things may still change to address bugs. Beta modules are subject to the beta allowed changes policy.

But, if we know we're going to have to do this, we probably should hold off on beta status until this is resolved.

catch’s picture

Agreed it's better to make planned changes before beta given we already know about them.

mikeryan’s picture

Status: Postponed » Reviewed & tested by the community

Per the discussion at https://www.drupal.org/node/2225717#comment-11805053 forward, the feeling is that the BC impact of #2225717: Add config translation support to migrations and implement for Drupal 6 user profile fields is minimal enough that it needn't block this.

xjm’s picture

Great!

I have updated https://www.drupal.org/core/experimental with this and we'll mention it in the release notes of both the minor and the patch release.

I also think Migrate API stabilizing is a big enough deal to merit a g.d.o/core announcement; anyone want to draft one?

xjm’s picture

(Policy decisions deserve issue credit too!)

xjm’s picture

Status: Reviewed & tested by the community » Fixed

@Gábor Hojtsy got this posted for us: https://groups.drupal.org/node/515916

Yay!

Gábor Hojtsy’s picture

Yay, I was about to circle back here :D Thanks for all the improvements to the text @xjm and the input @mikeryan :)

Status: Fixed » Closed (fixed)

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