MarcusBarnes / mik

The Move to Islandora Kit is an extensible PHP command-line tool for converting source content and metadata into packages suitable for importing into Islandora (or other digital repository and preservations systems).
GNU General Public License v3.0
34 stars 11 forks source link

Limit option causes errors, wrong number of packages #445

Closed bondjimbond closed 6 years ago

bondjimbond commented 6 years ago

Here's what happens whenever I run MIK with a limit option.

Brandons-Macbook:mik Brandon$ ./mik --config doh.ini --limit 10

Commencing MIK. Creating 10 Islandora ingest packages. Please be patient. ======================================================================> 100%

Fatal error: Uncaught exception 'Exception' with message 'The current is greater than the total.' in /Users/Brandon/mik/vendor/league/climate/src/TerminalObject/Dynamic/Progress.php:97 Stack trace:

0 /Users/Brandon/mik/mik(303): League\CLImate\TerminalObject\Dynamic\Progress->current(11)

1 {main}

thrown in /Users/Brandon/mik/vendor/league/climate/src/TerminalObject/Dynamic/Progress.php on line 97

And 11 packages are created, instead of the 10 that are requested.

mjordan commented 6 years ago

@bondjimbond how many total items in your fetcher's input?

bondjimbond commented 6 years ago

You mean how many items total exist in the CSV file? Around 395 for this run, 999 for the previous run. Both runs had this result.

mjordan commented 6 years ago

OK, thanks, I'll take a look. Does the error happen with other values of --limit such as 20, 30, etc.?

bondjimbond commented 6 years ago

Confirmed: other values create same result.

-- limit X results in error message, and number of packages generated = X+1.

mjordan commented 6 years ago

OK, thanks. I suspect someone, probably me, introduced an error in the --limit option sometime in the recent past.

mjordan commented 6 years ago

Hm. I'm beginning to suspect this is an issue with a third-party module, either the CSV reader or the progress bar library, because I checkout out code all the way back to March of this year and the error occurs there too (and it's impossible it would have gone undetected since March). I'll see if I can replicate this on Windows, which does not use the progress bar, tomorrow at work.

mjordan commented 6 years ago

Plot thickens. Just tested this on Windows. No errors, but although I specified a limit of 10 (-l 10) MIK produced 15 packages.

mjordan commented 6 years ago

Continuing to investigate. This error doesn't happen with OAI-PMH toolchains.

bondjimbond commented 6 years ago

You got 15 packages out of a limit 10? Very weird. Every run I've tried has been x+1.

Another thing on the Limit topic... This may merit its own issue, depending on your response. I'm wondering about what would be considered "expected behaviour". If I set --limit 5 in my test run, and objects at lines 1 and 4 are "problem records", MIK will skip over them and generate packages for 2, 3, 5, 6, 7, and 8 (because of the x+1 problem).

My expectation was that MIK should produce packages for only 2, 3, and 5, showing that objects 1 and 4 errored out... that is, my interpretation of the --limit 5 parameter is "Try the first five objects and return the results". MIK's interpretation is "Run MIK, and return 5 successful packages."

Both interpretations are valid, but what would you think is the average user's expectation?

mjordan commented 6 years ago

Closing, thanks for testing.