After upgrading to migrate_tools 4.1 from 4.0, I'm running the following command:

drush migrate-import d7_user --limit=100;

However I'm receiving the following error:

The "--limit" option does not accept a value.

I've tried --limit="100 items" and a couple of other things but to no avail.

I'm running Drupal 8.6.5, Drush 9.5.2.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ThomWilhelm created an issue. See original summary.

merlin06’s picture

I can confirm that after upgrading to migrate_tools 4.1 from 4.0 and running the following command I get a similar issue:
`drush migrate:import --group=scamwatch`
Error:
The "--group" option does not accept a value.

edysmp’s picture

me too

jeffwpetersen’s picture

I have also encountered this.

Alex Monaghan’s picture

Seeing the same issue with group, however, my test script has group followed by limit and don't get an error about limit.

Drush is 9.4.0
PHP 7.3 RC4

Upgraded yesterday from 8.6.4 to 8.6.7 which also updated migrate_tools & migrate_plus to 4.1

I have a large and complex database that needs to be re-structured, it takes over a day to run the full import so could really do with getting this working so I can do much smaller test chunks again. Anything I can provide to help, just shout.

KarenS’s picture

There is a new Drush option added in this release --idlist. I think something about that new option has broken things.

Alex Monaghan’s picture

I removed 4.1 (migrate_tools & plus) and installed 4.0 and the migrate is running again on 8.6.7 as it did on 8.6.4

KarenS’s picture

Another interesting clue, I tried this and get the error message:

drush migrate:import --group=drupal_7

But the following works:

drush migrate:import --group="drupal_7"

xurizaemon’s picture

I attempted to reproduce this against 8.x-4.x-dev and found that both the --limit and --group parameters worked as expected on a test site where I installed Migrate Tools 8.x-4.x-dev. I then installed Migrate Tools 8.x-4.1 and found I was able to reproduce it.

The behaviour then seemed to flip on and off, and as far as I can tell for me it's surfacing after eg module enable/disable, then disappearing after a few Drush commands. I'm aware this is not much help as far as reproduction goes :(

Setup:

  • drush status & pm:list
  • Drupal 8.6.7, Drush 9.4.0
  • Installed Migrate Tools 8.x-4.x-dev from d.o git via Composer (drupal/migrate_tools:dev-8.x-4.x)
  • Enabled migrate_source_csv and csv_source_test
  • Created a test.csv with three rows of three columns at web/sites/default/files/test.csv
  • For the final tag test below, I updated the configuration of csv_source_test migration to include tags 'Test' and OtherTest

First test, 8.x-4.x-dev

Repro test for limit parameter:

vagrant@ds2018:/vagrant$ drush migrate:import csv_source_test --limit=1
 [notice] Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - done with 'csv_source_test'

Correct: Source contains three rows, but limited import to a single row.

Repro test for group parameter:

vagrant@ds2018:/vagrant$ drush migrate:status
 ---------------------------------------- ----------------- -------- ------- ---------- ------------- --------------------- 
  Group                                    Migration ID      Status   Total   Imported   Unprocessed   Last Imported        
 ---------------------------------------- ----------------- -------- ------- ---------- ------------- --------------------- 
  CSV source plugin edit test (csv_test)   csv_source_test   Idle     3       1          2             2019-01-19 07:59:39  
                                                                                                                            
  Default (default)                        fruit_terms       Idle     3       0          3                                  
 ---------------------------------------- ----------------- -------- ------- ---------- ------------- --------------------- 
vagrant@ds2018:/vagrant$ drush migrate:status --group=csv_test
 ---------------------------------------- ----------------- -------- ------- ---------- ------------- --------------------- 
  Group                                    Migration ID      Status   Total   Imported   Unprocessed   Last Imported        
 ---------------------------------------- ----------------- -------- ------- ---------- ------------- --------------------- 
  CSV source plugin edit test (csv_test)   csv_source_test   Idle     3       1          2             2019-01-19 07:59:39  
 ---------------------------------------- ----------------- -------- ------- ---------- ------------- --------------------- 

Correct: Two migrations are available, group parameter limits to just that group for migrate:status command.

Repro test for migrate:import group parameter:

vagrant@ds2018:/vagrant$ drush migrate:import --group=csv_test
 [notice] Processed 2 items (2 created, 0 updated, 0 failed, 0 ignored) - done with 'csv_source_test'

Correct: Only the migration from csv_source_test group was run.

Repro test for migrate:import tag parameter:

vagrant@ds2018:/vagrant$ drush migrate:import --tag=Test
 [notice] Processed 0 items (0 created, 0 updated, 0 failed, 0 ignored) - done with 'csv_source_test'
vagrant@ds2018:/vagrant$ drush migrate:import --tag=OtherTest
 [notice] Processed 0 items (0 created, 0 updated, 0 failed, 0 ignored) - done with 'csv_source_test'

Second test, 8.x-4.1

Then I downgraded from 8.x-4.x-dev to 8.x-4.1 and was able to reproduce the reported issues.

vagrant@ds2018:/vagrant$ composer remove migrate_tools
migrate_tools is not required in your composer.json and has not been removed
No patches supplied.
> DrupalProject\composer\ScriptHandler::checkComposerVersion
Package "migrate_tools" listed for update is not installed. Ignoring.
Loading composer repositories with package information
Updating dependencies (including require-dev)                
^C
vagrant@ds2018:/vagrant$ composer remove drupal/migrate_tools
No patches supplied.
> DrupalProject\composer\ScriptHandler::checkComposerVersion
Dependency "drupal/core" is also a root requirement, but is not explicitly whitelisted. Ignoring.
Loading composer repositories with package information
Updating dependencies (including require-dev)                
Package operations: 0 installs, 0 updates, 2 removals
  - Removing drupal/migrate_tools (dev-8.x-4.x)
Deleting web/modules/contrib/migrate_tools - deleted
  - Removing drupal/migrate_plus (4.1.0)
Deleting web/modules/contrib/migrate_plus - deleted
Writing lock file
Generating autoload files
> DrupalProject\composer\ScriptHandler::createRequiredFiles
vagrant@ds2018:/vagrant$ drush migrate:import --tag=OtherTest

  The "--tag" option does not accept a value.  
                                               
migrate:import [--all] [--group] [--tag] [--limit] [--feedback] [--idlist] [--idlist-delimiter] [--update] [--force] [--execute-dependencies] [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-d|--debug] [-y|--yes] [--no] [--remote-host REMOTE-HOST] [--remote-user REMOTE-USER] [-r|--root ROOT] [-l|--uri URI] [--simulate] [--pipe] [-D|--define DEFINE] [--notify [NOTIFY]] [--druplicon] [--xh-link XH-LINK] [--]  [] []...

vagrant@ds2018:/vagrant$ drush migrate:import --group=csv_test

  The "--group" option does not accept a value.  
                                                 
migrate:import [--all] [--group] [--tag] [--limit] [--feedback] [--idlist] [--idlist-delimiter] [--update] [--force] [--execute-dependencies] [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-d|--debug] [-y|--yes] [--no] [--remote-host REMOTE-HOST] [--remote-user REMOTE-USER] [-r|--root ROOT] [-l|--uri URI] [--simulate] [--pipe] [-D|--define DEFINE] [--notify [NOTIFY]] [--druplicon] [--xh-link XH-LINK] [--]  [] []...

vagrant@ds2018:/vagrant$ drush migrate:import csv_source_test --limit=1

  The "--limit" option does not accept a value.  
                                                 
migrate:import [--all] [--group] [--tag] [--limit] [--feedback] [--idlist] [--idlist-delimiter] [--update] [--force] [--execute-dependencies] [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-d|--debug] [-y|--yes] [--no] [--remote-host REMOTE-HOST] [--remote-user REMOTE-USER] [-r|--root ROOT] [-l|--uri URI] [--simulate] [--pipe] [-D|--define DEFINE] [--notify [NOTIFY]] [--druplicon] [--xh-link XH-LINK] [--]  [] []...

Huh?

I can now reproduce the issue, but I can't explain how it arose after downgrading from 8.x-4.x-dev to 8.x-4.1 because they appear to be the same code? (git diff 8.x-4.1..8.x-4.x shows no changes.)

And then moments later the behaviour vanished. Having made no further changes, I can't reproduce it any more.

makkus183’s picture

Issue summary: View changes

Seeing the same issue with --group:

drush migrate-import --group=top_import --update
drush migrate-import --group="top_import" --update

In both options i get:

 The "--group" option does not accept a value.
robcast’s picture

I also see the "option does not accept a value" problem with the --feedback option in 4.1.

Sometimes it helps to put the value in quotes or remove the quotes. Sometimes it starts to happen after a drush cr...

hudri’s picture

hudri’s picture

Seems it is caused by update of Plus v4.1 and not by update of Tools 4.1. In my case my exisiting migrations do work with Tools v4.1 and Plus v4.0, but stop working as soon as I update Plus to v4.1

arnested’s picture

Component: Code » Drush commands
FileSize
2.92 KB

The issues seems to be in the way https://github.com/consolidation/annotated-command parses the drush command options.

Attached is a patch correcting this.

arnested’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 14: migration-tools_fix-drush-options_3024399-9.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

arnested’s picture

Status: Needs work » Needs review
FileSize
2.86 KB

Code style fixes to patch.

Status: Needs review » Needs work

The last submitted patch, 17: migrate-tools_fix-drush-options_3024399-17.patch, failed testing. View results

mpp’s picture

I can confirm the same behavior:

$ drush cr

$ drush migrate:status --group=migrate_sg
 The "--group" option does not accept a value.

$ drush migrate:status --group=migrate_sg
------------------------ -------------------------- ----------- ------- ---------- ------------- ---------------------
 Group                    Migration ID               Status      Total   Imported   Unprocessed   Last Imported
------------------------ -------------------------- ----------- ------- ---------- ------------- ---------------------
...
moshe weitzman’s picture

The latest patch looks sane to me. You have to declare your options both in the doxygen and give default values in the function signature.

arnested’s picture

Status: Needs work » Needs review
FileSize
6.7 KB

Improved patch added. This will hopefully parse the tests.

Status: Needs review » Needs work

The last submitted patch, 21: migrate-tools_fix-drush-options_3024399-21.patch, failed testing. View results

arnested’s picture

Status: Needs work » Needs review
FileSize
5.54 KB

Failed again (of course now I look at it again).

Let's try this patch., then...

arnested’s picture

Priority: Normal » Major

Yay. Tests succeeded.

I'm raising the priority to 'Major' as the drush commands are not really working without this fix.

chaitanya.maili’s picture

I have tested the above patch, it worked fine for me.

I feel this is good to merge.

daveiano’s picture

The path in #23 fixes the problem for me, too.

However if the migration fails (due to errors in my own custom logic in my migration.yml), it gives a strange error message, which I first misunderstood as the previous --limit error:

drush migrate error

I don't know if this is related, but you can see the migration fails (100 of 100 items failed), but the error message says 'Migration - 100 failed'

EDIT: Forget what I was saying, the message says how many items fail in the migration process, everything is correct.

kevinquillen’s picture

Same here. I can reproduce this by doing a cache clear. The first call will fail. The subsequent calls do not. Clearing the cache again will cause the error.

Here is the output:

➜  mysite git:(migrate-poc) ahoy drush @mysite.local cr                    
 [success] Cache rebuild complete.
➜  mysite git:(migrate-poc) ahoy drush @mysite.local migrate:import --group=mygroup --limit=5 --feedback

                                                 
  The "--group" option does not accept a value.  
                                                 

migrate:import [--all] [--group] [--tag] [--limit] [--feedback] [--idlist] [--idlist-delimiter] [--update] [--force] [--execute-dependencies] [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-d|--debug] [-y|--yes] [--no] [--remote-host REMOTE-HOST] [--remote-user REMOTE-USER] [-r|--root ROOT] [-l|--uri URI] [--simulate] [--pipe] [-D|--define DEFINE] [--notify] [--druplicon] [--xh-link XH-LINK] [--] <command> [<migration_names>] [<options>]...


➜  mysite git:(migrate-poc) ahoy drush @mysite.local migrate:import --group=mygroup --limit=5 --feedback
 [notice] Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'working_paper'
 [notice] Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'working_paper'
 [notice] Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'working_paper'
 [notice] Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - continuing with 'working_paper'
 [notice] Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - done with 'working_paper'
➜  mysite git:(migrate-poc):

Is there something about the cache rebuild that causes this?

kevinquillen’s picture

The supplied patch fixed the group and limit issue, but introduces a new one:

The "--feedback" option requires a value.

In the patch, setting 'feedback' to FALSE corrects this behavior. I could no longer reproduce my issue, that said, I am only using the group, limit, and feedback options in my work so far.

public function import($migration_names = '', array $options = [
    'all' => FALSE,
    'group' => self::REQ,
    'tag' => self::REQ,
    'limit' => self::REQ,
    'feedback' => FALSE,
    'idlist' => self::REQ,
    'idlist-delimiter' => ':',
    'update' => FALSE,
    'force' => FALSE,
    'execute-dependencies' => FALSE,
  ])

The console output also looks correct too for the help after making that change:

migrate:import [--all] [--group GROUP] [--tag TAG] [--limit LIMIT] [--feedback] ...

The initial patch would have it as [--feedback FEEDBACK] and any value passed to it would cause a fatal error.

I say this needs some work to ensure that the options and flags are being handled properly.

kevinquillen’s picture

Status: Needs review » Needs work
arnested’s picture

But as far as I can tell feedback should take a value

   * @option feedback Frequency of progress messages, in items processed

I never used the feedback option before, but when testing with the patch it appears to do what it is supposed to do
drush migrate:import --all --feedback=10

robcast’s picture

I always used feedback with a value --feedback=500 and I think this is the most useful option. I don't want to see 10000 lines of feedback for an import of 10000 rows but 10 or 20 are fine.

In the past feedback without a value was the same as --feedback=1 but I could live without that shortcut.

kevinquillen’s picture

Got it. Didn't know feedback took a value. That makes sense now.

arnested’s picture

I had a look in https://github.com/consolidation/annotated-command to see if we could specify that behaviour but it does not appear to be the case.

It might be possible to abuse InputOption::VALUE_OPTIONAL since true will be cast to 1, but I'm not sure going down that path is wise.

kehan’s picture

Status: Needs work » Reviewed & tested by the community

I think we can say that #23 does work, and feedback requires an option.

Alice Heaton’s picture

Same issue, patch at #23 works for me

juampynr’s picture

Patch at #23 works for me too.

ressa’s picture

Thanks @arnested, #23 also works for me.

Jorge Navarro’s picture

#23 works

jcmartinez’s picture

#23 Works!

steveoriol’s picture

#23 works on D8.7.0, Drush 9.6.2 and drupal/migrate_tools:4.1.0

DanChadwick’s picture

#23 fixed it for me too.

mpp’s picture

Rtbc++

pixlkat’s picture

RTBC++ for me as well -- #23 fixed my issue.

MKorostoff’s picture

Patch is #23 works for me

  • heddn committed 1513fb7 on 8.x-4.x authored by arnested
    Issue #3024399 by arnested, daveiano, ThomWilhelm, robcast, mpp, hudri,...
heddn’s picture

Status: Reviewed & tested by the community » Fixed
mpp’s picture

Awesome, thanks all!

  • heddn committed c62ef84 on 8.x-4.x
    Revert "Issue #3024399 by arnested, daveiano, ThomWilhelm, robcast, mpp...
heddn’s picture

Status: Fixed » Needs work

This unfortunately broke HEAD. We'll need to figure out how to fix that. For now I've reverted.

andypost’s picture

Issue tags: +Needs tests

Testing should be extended

heddn’s picture

This could also use an IS update. This might not be an issue any more. I went to reproduce the issue as defined in the IS and I'm not able to do so:

heddn@drupal-web:/var/www/html$ vendor/bin/drush mim --limit 2 migrate_csv
 [notice] Processed 2 items (2 created, 0 updated, 0 failed, 0 ignored) - done with 'migrate_csv'
heddn@drupal-web:/var/www/html$ vendor/bin/drush mim migrate_csv --limit 3
 [notice] Processed 3 items (3 created, 0 updated, 0 failed, 0 ignored) - done with 'migrate_csv'
heddn@drupal-web:/var/www/html$ vendor/bin/drush ms
 ------------------- -------------- -------- ------- ---------- ------------- ---------------------
  Group               Migration ID   Status   Total   Imported   Unprocessed   Last Imported
 ------------------- -------------- -------- ------- ---------- ------------- ---------------------
  Default (default)   migrate_csv    Idle     15      5          10            2019-08-07 21:09:25
 ------------------- -------------- -------- ------- ---------- ------------- ---------------------
andypost’s picture

Iirc it was failed to parse idlist passed but then it went other way

tonytheferg’s picture

I was getting The "--tag" option does not accept a value. and applied #23 and it worked. @heddn, Just curious what "breaking HEAD" means?

Thanks!

BrightBold’s picture

I also got this error with --idlist and the patch in #23 fixed it. Not sure why it's breaking HEAD but it seems to work locally.

(FWIW, I don't think this is a problem with earlier versions of Drush. I had used --idlist multiple times successfully earlier this weekend under 8.x-4.1 with no problems, and then realized my Lando container had Drush pinned at 8.1, which isn't supported with Drupal 8.7, so I upgraded to Drush 9 and that's when the problem occurred.)

heddn’s picture

Status: Needs work » Needs review
FileSize
6.13 KB

Here's the patch in #23 re-rolled against HEAD. It should fail tests.

Status: Needs review » Needs work

The last submitted patch, 55: 3024399-55.patch, failed testing. View results

APolitsin’s picture

Priority: Major » Critical

The module does not work for most user cases and there is no possibility of downgrade. I think this is a critical problem.

ThomWilhelm’s picture

Priority: Critical » Major

Whilst I agree this is a really important issue, it's not critical as per priority guidelines: https://www.drupal.org/core/issue-priority

You can apply the patch from this ticket for example, so that's a known workaround.

Balu Ertl’s picture

Title: The "--limit" option does not accept a value. » The "--limit" option does not accept a value
labboy0276’s picture

Thanks for the patch, it worked. I usually always got this after a drush cr. My original work around was to run drush ms and then the commands worked ok again for some reason.

ilaz’s picture

The "--update" option does not accept a value.

It show this error with the patch from #55.

I changed that patch with 'update' => self::REQ, and now it works.

heddn’s picture

Did some more debugging on this today. And the int values from `self::REQ` are passed along. And those keep getting passed around. And break things. We need the actual string values from those things. So, the test failures here are legit and should be fixed.

heddn’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
11.92 KB
16.33 KB

Let's see how this goes. It has more test coverage. And most importantly, it reproduces the bug and tests it several different ways.

heddn’s picture

FileSize
1 KB
16.6 KB

Let's add a little more test coverage.

heddn’s picture

Issue tags: -Needs issue summary update
FileSize
16.63 KB
1.34 KB

And the migrate:status command was broken. Let's fix that.

heddn’s picture

Issue tags: +Needs manual testing

Tagging for some manual testing. It seems to work for me, but :shrug: I'd like some feedback from the community.

moshe weitzman’s picture

Self::REQ means that when the option is presented, it must be presented with a value. This is appropriate for --limit and others.

The benefit of this is that

  • Console can error when the option is presented without a value.
  • Help indicates when a value is required or not via square brackets.
heddn’s picture

FileSize
1.93 KB
16.63 KB

Fixing #67

moshe weitzman’s picture

Status: Needs review » Needs work

Pretty close. I dont see value in fiddling like

+     $group_names = $options['group'] ?? NULL;

+    $all = isset($options['all']);

$options['group'] will be a string or NULL when you receive it. Seems like this code provides no benefit. There are lot of new isset() and I they could be just $options['foo'] instead of isset($options['foo'])

$options['all'] can only be TRUE or FALSE. You cant get passed anything else.

heddn’s picture

Status: Needs work » Needs review
FileSize
12.98 KB
22.05 KB

This should fix #69.

moshe weitzman’s picture

- * @option idlist-delimiter The delimiter for records, defaults to ':'

FYI Help shows default value automatically.

It may be helpful in the tests to pass --format=json so you have machine parseable output. At least for migrate:status command. Just a thought.

  • heddn committed 09e28fd on 8.x-4.x
    Issue #3024399 by heddn, arnested, daveiano, moshe weitzman, mpp,...
heddn’s picture

Status: Needs review » Fixed
Issue tags: -Needs manual testing

OK, taking the feedback from #71, merged it with the previous patch and committed this thing. Thanks everyone for your feedback and persistence on seeing this land.

maximpodorov’s picture

What about the new release?

Status: Fixed » Closed (fixed)

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

codebymikey’s picture

This commit breaks the requirements when --execute-dependencies is specified.

$migration->get('requirements');

is not the same as

$definition = $migration->getPluginDefinition();
$required_migrations = $definition['requirements'] ?? [];

The requirements property is built and set in the \Drupal\migrate\Plugin\MigrationPluginManager::createInstances function.

Trying to follow the reason behind the change leads to this.

Since there's no interface method available to retrieve that property, the only viable solution is to use MigrationInterface::getMigrationDependencies(), and build a similar list of dependency ids.

I'll create a separate issue for this bug so it's easier to manage.