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

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.

me too

I have also encountered this.

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.

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

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

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"

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.

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.

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...

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

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.

Code style fixes to patch.

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
------------------------ -------------------------- ----------- ------- ---------- ------------- ---------------------
...

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.

Improved patch added. This will hopefully parse the tests.

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

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

Yay. Tests succeeded.

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

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

I feel this is good to merge.

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:

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.

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?

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.

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

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.

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

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.

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

Same issue, patch at #23 works for me

Patch at #23 works for me too.

Thanks @arnested, #23 also works for me.

#23 works

#23 Works!

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

#23 fixed it for me too.

Rtbc++

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

Patch is #23 works for me

Awesome, thanks all!

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

Testing should be extended

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
 ------------------- -------------- -------- ------- ---------- ------------- ---------------------

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

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!

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.)

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

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

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.

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.

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.

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

Let's add a little more test coverage.

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

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

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.

Fixing #67

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.

This should fix #69.

- * @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.

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.

What about the new release?

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.