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

Issue 156 #350

Closed jpeak5 closed 7 years ago

jpeak5 commented 7 years ago

Github issue: https://github.com/MarcusBarnes/mik/issues/156

What does this Pull Request do?

This PR offers an initial implementation of shutdown hooks, a feature described in #156.

What's new?

Adds a .ini array, [WRITER]['shutdownhooks'] that should contain commands to be executed just before mik exits. After the postwritehooks loop completes, the elements of this array will be invoked, one after the other.

Possible problems

As with post-write hooks, the shutdown hooks loop begins without checking to be sure the post-write processes have completed, therefore, anything that relies on the output of the post-write stage will need to do it's own completion-checking (perhaps a log or other artifact left by post-write to signify completion).

How should this be tested?

Add the following to your .ini. When mik exits the config file should have a current last-modified time.

[WRITER]
shutdownhooks[] = "touch"

For a less-trivial test, try the included xslt runner.

[WRITER]
shutdownhooks[] = "/usr/bin/php extras/scripts/shutdownhooks/apply_xslt_with_saxon.php"

which will require some additional configuration:

[XSLT]
working_dir   = 'hnoc-delcroix/hnoc-xslt'
step_thru     = TRUE
stylesheets[] = 'extras/lsu/xsl/titleNonSort.xsl'
stylesheets[] = 'extras/lsu/xsl/subjectSplit.xsl'
stylesheets[] = 'extras/lsu/xsl/dateCreatedSplit.xsl'
stylesheets[] = 'extras/lsu/xsl/locationMerge.xsl'
stylesheets[] = 'extras/lsu/xsl/blankNodes.xsl'
stylesheets[] = 'extras/lsu/xsl/OrderedTemplates.xsl'

Additional Notes:

Documentation snippet:

Shutdown hooks

Similar to post-write hooks, but as the name implies, shutdown hooks run just before mik exits. Shutdown hooks also differ in that they are run sequentially. That is, the first shutdown hook must exit before the second is invoked.

Register shutdown hooks in .ini in the [WRITER] section:

 shutdownhooks[] = "/usr/bin/php extras/scripts/shutdownhooks/shutitdown.php"

Interested parties

@mjordan @MarcusBarnes

mjordan commented 7 years ago

@jpeak5 nice! I can give this a smoketest.

mjordan commented 7 years ago

Looks good. I can verify your "touch" example. I didn't scaffold up to test the XSLT case, but instead I created my own small two shutdown scripts, which I've registered in my .ini file like this:

[WRITER]
shutdownhooks[] = "/tmp/test1.sh"
shutdownhooks[] = "/tmp/test2.sh"

My two scripts are:

#!/bin/bash
echo "I am the output from a test script" >> /tmp/hey.txt

and

#!/bin/bash
echo "I am the output from a second test script" >> /tmp/hey.txt

After runnig mik, I can confirm that /tmp/hey.txt contains the two expected lines:

I am the output from a test script
I am the output from a second test script

So, they executed in the registered order. Nice!

The only thing I encountered that I didn't expect was the mik appears to be no longer executable; @jpeak5 could you make sure it is in your PR?

mjordan commented 7 years ago

Not to complicate things, but I wonder if we should be implementing the registered shutdown functions via PHP's register_shutdown_function() so they get executed even if mik crashes? Not sure there is a use case for that, just raising the question.

jpeak5 commented 7 years ago

Interesting question, and one I'm afraid I am not knowledgeable enough to debate. Having skimmed the php docs just now, it seems like it would be trivial to write, and arguably more php-ish...

I would need to test, but I'd be glad to take that route if it seems better

mjordan commented 7 years ago

@jpeak5 thanks for the permission update. Just tested, works.

mjordan commented 7 years ago

@jpeak5 let me confirm that register_shutdown_function() is in fact called when the script "crashes". I'll have to figure out what that means first, however. :smile:

In the meantime, I'm happy to merge this PR since your implementation works well as is. We can always open a new issue for the register_shutdown_function() part. @MarcusBarnes what are your thoughts?

MarcusBarnes commented 7 years ago

I haven't had a chance to test, but since it introduces functionality useful for using MIK as part of a digitization pipeline, I'm happy to have this merged. We can enhance this feature later. Thank you @jpeak5 for this contribution.

jpeak5 commented 7 years ago

Thanks gentlemen...!

mjordan commented 7 years ago

OK, all is good, I tested so I'll merge now. Thanks again @jpeak5, we really appreciate your contributions to MIK!

mjordan commented 7 years ago

I'll document this new feature on the wiki.

jpeak5 commented 7 years ago

Thanks very much, Mark!