|
Description
Kyle M Hall (khall)
2020-06-11 15:17:11 UTC
Created attachment 105767 [details] [review] Bug 25716: Add ability to specify additional options in koha-conf.xml for z3950_responder.pl when using koha-z3950-responder The z39.50 responder has a number of command line options that are not accessible if using the debian scripts to control it. We should be able to set those options in the koha conf file to be passed to the script itself. Test Plan: 1) Apply this patch 2) Copy your kohaclone's koha-z3950-responder to /usr/sbin/koha-z3950-responder if necessary 3) Add "<z3950_responder_options>--add-item-status k</z3950_responder_options>" inside your <config> block in your koha-conf.xml file 4) Use koha-z3950-responder to start/restart the z39.50 responder, note the item status is now in subfield k! Hi Kyle, There needs to be a check before doing the xmlstarlet here, as the responder won't start if the config option doesn't exist in koha-conf.xml. This will be a problem for upgrading libraries who don't yet have the config options. Created attachment 114166 [details] [review] Bug 25716: Don't exit if config doesn't exist in Koha conf file Created attachment 120178 [details] [review] Bug 25716: Don't exit if config doesn't exist in Koha conf file There's an XML problem I noticed with the syntax highlighter:
✔ ~/git/koha [qa_25716 ↑·2|✚ 1⚑ 33]
12:57 $ xmllint etc/koha-conf.xml
etc/koha-conf.xml:271: parser error : Double hyphen within comment: <!--
<z3950_responder_options>
<z3950_responder_options>--add-item-status k -t 5</z3950_responder_options>
^
I suggest we move this into the z3950/ files
It is not breaking xmlstarlet, so not an issue. Back to NSO. I sign this off (we have had this in production for many months) - however my devbox is broken and I do not have a spoon to get it working right now. Created attachment 125095 [details] [review] Bug 25716: Add ability to specify additional options in koha-conf.xml for z3950_responder.pl when using koha-z3950-responder The z39.50 responder has a number of command line options that are not accessible if using the debian scripts to control it. We should be able to set those options in the koha conf file to be passed to the script itself. Test Plan: 1) Apply this patch 2) Copy your kohaclone's koha-z3950-responder to /usr/sbin/koha-z3950-responder if necessary 3) Add "<z3950_responder_options>--add-item-status k</z3950_responder_options>" inside your <config> block in your koha-conf.xml file 4) Use koha-z3950-responder to start/restart the z39.50 responder, note the item status is now in subfield k! Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 125096 [details] [review] Bug 25716: Don't exit if config doesn't exist in Koha conf file Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 125481 [details] [review] Bug 25716: Add ability to specify additional options in koha-conf.xml for z3950_responder.pl when using koha-z3950-responder The z39.50 responder has a number of command line options that are not accessible if using the debian scripts to control it. We should be able to set those options in the koha conf file to be passed to the script itself. Test Plan: 1) Apply this patch 2) Copy your kohaclone's koha-z3950-responder to /usr/sbin/koha-z3950-responder if necessary 3) Add "<z3950_responder_options>--add-item-status k</z3950_responder_options>" inside your <config> block in your koha-conf.xml file 4) Use koha-z3950-responder to start/restart the z39.50 responder, note the item status is now in subfield k! Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125482 [details] [review] Bug 25716: Don't exit if config doesn't exist in Koha conf file Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> I can't really fail this.. but I would have preferred to see it as options in the z3950 specific config file rather than this somewhat obscure pass a param into the config.. it feels weird having `--add-item-status k--` inside a config line But.. the code does indeed work and although we get a noisy warning about XML issues in the qa scripts it doesn't appear to actually cause an issue with the feature. Passing QA. Shouln't it be more in etc/z3950/config.xml? Or, isn't actually more something for an environment variable? Created attachment 142520 [details] [review] Bug 25716: Add ability to specify additional options in koha-conf.xml for z3950_responder.pl when using koha-z3950-responder The z39.50 responder has a number of command line options that are not accessible if using the debian scripts to control it. We should be able to set those options in the koha conf file to be passed to the script itself. Test Plan: 1) Apply this patch 2) Copy your kohaclone's koha-z3950-responder to /usr/sbin/koha-z3950-responder if necessary 3) Add "<z3950_responder_options>--add-item-status k</z3950_responder_options>" inside your <config> block in your koha-conf.xml file 4) Use koha-z3950-responder to start/restart the z39.50 responder, note the item status is now in subfield k! Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 142521 [details] [review] Bug 25716: Don't exit if config doesn't exist in Koha conf file Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 142522 [details] [review] Bug 25716: (QA follow-up) Move additional options to etc/z3950/config.xml This followup moves the configuration to the z3950 etc file, either the default and or the custom file is used as per the existing script code. In addition, the options and be set using an environment variable named Z3950_ADDITIONAL_OPTS. Pushed to master for 22.11. Nice work everyone, thanks! Created attachment 142577 [details] [review] Bug 25716: (QA follow-up) Make sure a comment doesn't break tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> |