Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a migration-destroy-id-map command #845

Closed

Conversation

tstoeckler
Copy link
Contributor

See #844

@weitzman
Copy link
Member

Too specialized for core Drush IMO. I propose putting these commands in 8.x-1.x branch of Migrate contrib project.

@weitzman weitzman closed this Sep 26, 2014
@benjy
Copy link
Contributor

benjy commented Sep 28, 2014

Is there a guideline for what we're going to support in the migrate contrib module vs in core migrate?

If drush is just to support what is in core then what we already have is probably enough for drush and the rest should go to the migrate module?

@weitzman
Copy link
Member

I think that even the existing migrate command in drush should move to the
migrate project.

Core Drupal is a huge app and core drush can't maintain all the
integrations with it.

@benjy
Copy link
Contributor

benjy commented Sep 28, 2014

Well that would mean that either: a) the migrate command ends up in core or b) there is no way to do the core upgrade other than using the UI (without installing a contrib module)?

@weitzman
Copy link
Member

you would install vanilla d8, then enable migrate Contrib. I dont see a
problem with that.

@chx
Copy link
Contributor

chx commented Sep 28, 2014

So then why do we have migrate in core after your post to groups.drupal.org/core about the policy and this PR?

@benjy
Copy link
Contributor

benjy commented Sep 28, 2014

If that's the case we may as well move migrate back to contrib.

You didn't need a contrib module in D7 to upgrade from D6, why would you need one to upgrade from D7 to D8?

@chx
Copy link
Contributor

chx commented Sep 28, 2014

This is a new territory -- but the current battle plan is to make people at least able to test the migrate path with drush and if everything goes well then there will be UI and then we can move this to contrib. But until then, I have no idea where to put it short of convincing the core committers of allowing a drush command in core temporarily.

@weitzman
Copy link
Member

My motivation for putting migrate framework in core is to

  1. Remove update.php and all its complexity from core. Simplify our main app.
  2. Focus the energy of many people on a migrate solution versus in-place-update.

For me, the detail about where the drush commands live is not even close to the importance of those to things.

Using Migrate 8.x-1.x is a bonus for you guys. I can add you as Maintainers. You can update your Drush commands when you need to. Seems like there is a lot of Win there.

@chx
Copy link
Contributor

chx commented Sep 28, 2014

So, our problem is this: if you need to install migrate 8.x-1.x contrib to run any migration at all then again what's the point? We could've said that among the new things D8 does is there no is update path and use migrate in contrib, bye. Then we would need to rally people around the contrib project...

I am completely baffled this suggestion. Do you really not see how ridiculous this suggestion is?

@benjy
Copy link
Contributor

benjy commented Sep 29, 2014

@weitzman, I agree with your two points regarding the motives for migrate in core. However, it's bizarre to me that we'd have a migrate api and the D6 to D8 migrations in core without either a drush command or a UI to run them. Essentially, core code is unusable without contrib.

We should either commit the drush runner here or to core directly until the time comes for the UI to be put into core IMO.

@alexpott
Copy link
Contributor

alexpott commented Oct 7, 2014

@weitzman if we want people to be able to test easily saying get drush and core is doable. Saying get this other thing sucks. Of course we can put the commands in core but they belong there even less than drush. Also I buy the argument that since migrate is D8's way to upgrade migrate core commands should probably be in core drush.

@catch56
Copy link

catch56 commented Oct 7, 2014

It was possible to get from Drupal 6 to Drupal 7 using core and drush, so I think it's reasonable for people to be able to expect to do the same thing from Drupal 6/7 to Drupal 8.

Additionally if we ever start requiring 8.x to 8.x migrations for some contrib module upgrade paths or similar, then migrate will increasingly replace drush updb for that too, and not having it available in drush will make arguing for that harder.

@weitzman
Copy link
Member

OK, I acknowledge that putting these commands into Drush core is useful for many people and many reasons. My sole remaining objection is maintenance.

  1. Need tests. See Drush's other tests for a general approach.
  2. These commands need a responsible, competent maintainer. I will support that person, but I can't be the lead maintainer of yet another Drush subsystem.

@weitzman weitzman reopened this Oct 16, 2014
@benjy
Copy link
Contributor

benjy commented Oct 16, 2014

I'm prepared to take on maintaining the drush migrate command. I'm already a core maintainer for migrate and I've worked on the D8 migrate command as well.

@tstoeckler
Copy link
Contributor Author

I (very) briefly talked to @weitzman about this at DrupalCon Amsterdam. After working with Migrate for a while now I am a bit less excited about such a command. The problem is that the ID map is not the only artefact a migration might leave behind. If your migration is already broadly working, but you still want to iron out some kinks, then the destination plugin will already save entities and whatnot into your database, which you will then want to clean up as well before doing the next test run.

In other words, Migrate in D8 core is missing a "rollback" feature. Being able to trigger a true rollback from Drush would be invaluable, but I'm not sure that's currently possible with the Drupal core Migrate functionality.

(The migrate-manifest command, though, is very useful regardless, and it would be awesome to keep it. Thanks @Benjy14 for stepping up!)

@chx
Copy link
Contributor

chx commented Oct 17, 2014

Rollback should be possible because of the logic in Drupal\migrate\Source::next and the whole id map is basically a 1:1 copy of migrate D7 but it's not tested or anything. Volunteers welcome :D

@weitzman
Copy link
Member

Thanks @Benjy14. That works for me. The first order of business is to add Drush tests for migrate command(s). Thats non-trivial I think. Ping me if needed and we can brainstorm.

@weitzman
Copy link
Member

weitzman commented Jan 8, 2016

migrate commands are in core drush anymore.

@weitzman weitzman closed this Jan 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants