remove_unused_authorities.pl script could be improved. At least : test mode more verbose, ES compatibility, add confirm arg
Created attachment 82558 [details] [review] Bug 21865: improve remove_unused_authorities.pl script remove_unused_authorities.pl script can be improved. This patch changes changes verbosity so than test mode can be used to know the autorities that are used and those that can be deleted. It also writes a line in output if limited authority type(s). This patch also removes the unused vars $thresholdmin and $thresholdmax. It also changes the query to use SQL with parameters for authority types. Test plan : 1) On a catalog create a new authority 2) Be sure catalog is well indexed 3) Run misc/migration_tools/remove_unused_authorities.pl -t 4) You will see the line : *** Testing only, authorities will not be deleted. *** 5) You will see lines of : authid=x type=y : used X time(s) 6) You will see the line for the authority created in 1) : authid=x type=y : can be deleted 7) You will see at the end : x authorities parsed y can be deleted because unused z unchanged because used 8) Run misc/migration_tools/remove_unused_authorities.pl 9) You don't see the line : *** Testing only, authorities will not be deleted. *** 10) You will see lines of : authid=x type=y : used X time(s) 11) You will see the line for the authority created in 1) : authid=x type=y : deleted 12) You will see at the end : x authorities parsed y deleted because unused z unchanged because used 13) Run misc/migration_tools/remove_unused_authorities.pl --auth NP --auth CO 14) You see the line : Restricted to authority type(s) : NP,CO.
Created attachment 82559 [details] [review] Bug 21865: ES compatiblity of remove_unused_authorities.pl The script misc/migration_tools/remove_unused_authorities.pl directly checks if Zebra search is OK. This patch changes so that this test is only if Zebra is the search engine. It also adds a test on the search off any authority number indexed (index 'an'). With Zebra its : an,alwaysmatches='' With ES its : an:* This test ensure that biblios records are indexed and that not all autorities will be deleted. Test plan: 1) On a catalog create a new authority 2) Use Zebra in systempreference SearchEngine 3) Stop Zebra server 4) Run misc/migration_tools/remove_unused_authorities.pl -c 5) The script does nothing and says : Zebra server seems not to be available. This script needs Zebra runs. 6) Restart Zebra server 7) Delete biblio index base 8) Run misc/migration_tools/remove_unused_authorities.pl -c 9) The script does nothing and says : Searching authority number in biblio records seems not to be available : an,alwaysmatches='' 10) Use ElasticSearch in systempreference SearchEngine 11) Delete biblio index base 12) Run misc/migration_tools/remove_unused_authorities.pl -c 13) The script does nothing and says : Searching authority number in biblio records seems not to be available : an:*
Created attachment 82560 [details] [review] Bug 21865: add confirm to remove_unused_authorities.pl script Like in most scripts in misc, add confirm argument to ensure script is not run without knowing what it does. Test plan: 1) Run misc/migration_tools/remove_unused_authorities.pl -h 2) You see help line for confirm 3) Run misc/migration_tools/remove_unused_authorities.pl 4) You see help and script does nothing 5) Run misc/migration_tools/remove_unused_authorities.pl -c 6) Script runs like wanted
Hi, with this code 40 if ( $want_help || !$confirm ) { you need to confirm even to do a test. Is that the intended behavior? May be "if ( $want_help || !($test || $confirm) ) {" Otherwise the script works well (tested using zebra)
(In reply to Bernardo Gonzalez Kriegel from comment #4) > Hi, > with this code > > 40 if ( $want_help || !$confirm ) { > > you need to confirm even to do a test. > Is that the intended behavior? > > May be "if ( $want_help || !($test || $confirm) ) {" > > Otherwise the script works well (tested using zebra) Indeed, good idea
Created attachment 100577 [details] [review] Bug 21865: allow test arg alone
Created attachment 100918 [details] [review] Bug 21865: improve remove_unused_authorities.pl script remove_unused_authorities.pl script can be improved. This patch changes changes verbosity so than test mode can be used to know the autorities that are used and those that can be deleted. It also writes a line in output if limited authority type(s). This patch also removes the unused vars $thresholdmin and $thresholdmax. It also changes the query to use SQL with parameters for authority types. Test plan : 1) On a catalog create a new authority 2) Be sure catalog is well indexed 3) Run misc/migration_tools/remove_unused_authorities.pl -t 4) You will see the line : *** Testing only, authorities will not be deleted. *** 5) You will see lines of : authid=x type=y : used X time(s) 6) You will see the line for the authority created in 1) : authid=x type=y : can be deleted 7) You will see at the end : x authorities parsed y can be deleted because unused z unchanged because used 8) Run misc/migration_tools/remove_unused_authorities.pl 9) You don't see the line : *** Testing only, authorities will not be deleted. *** 10) You will see lines of : authid=x type=y : used X time(s) 11) You will see the line for the authority created in 1) : authid=x type=y : deleted 12) You will see at the end : x authorities parsed y deleted because unused z unchanged because used 13) Run misc/migration_tools/remove_unused_authorities.pl --auth NP --auth CO 14) You see the line : Restricted to authority type(s) : NP,CO. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 100919 [details] [review] Bug 21865: ES compatiblity of remove_unused_authorities.pl The script misc/migration_tools/remove_unused_authorities.pl directly checks if Zebra search is OK. This patch changes so that this test is only if Zebra is the search engine. It also adds a test on the search off any authority number indexed (index 'an'). With Zebra its : an,alwaysmatches='' With ES its : an:* This test ensure that biblios records are indexed and that not all autorities will be deleted. Test plan: 1) On a catalog create a new authority 2) Use Zebra in systempreference SearchEngine 3) Stop Zebra server 4) Run misc/migration_tools/remove_unused_authorities.pl -c 5) The script does nothing and says : Zebra server seems not to be available. This script needs Zebra runs. 6) Restart Zebra server 7) Delete biblio index base 8) Run misc/migration_tools/remove_unused_authorities.pl -c 9) The script does nothing and says : Searching authority number in biblio records seems not to be available : an,alwaysmatches='' 10) Use ElasticSearch in systempreference SearchEngine 11) Delete biblio index base 12) Run misc/migration_tools/remove_unused_authorities.pl -c 13) The script does nothing and says : Searching authority number in biblio records seems not to be available : an:* Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 100920 [details] [review] Bug 21865: add confirm to remove_unused_authorities.pl script Like in most scripts in misc, add confirm argument to ensure script is not run without knowing what it does. Test plan: 1) Run misc/migration_tools/remove_unused_authorities.pl -h 2) You see help line for confirm 3) Run misc/migration_tools/remove_unused_authorities.pl 4) You see help and script does nothing 5) Run misc/migration_tools/remove_unused_authorities.pl -c 6) Script runs like wanted Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 100921 [details] [review] Bug 21865: allow test arg alone Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tested with zebra, no errors.
(In reply to Bernardo Gonzalez Kriegel from comment #11) > Tested with zebra, no errors. And tested with Elasticsearch, no errors
Created attachment 102836 [details] [review] Bug 21865: improve remove_unused_authorities.pl script remove_unused_authorities.pl script can be improved. This patch changes changes verbosity so than test mode can be used to know the autorities that are used and those that can be deleted. It also writes a line in output if limited authority type(s). This patch also removes the unused vars $thresholdmin and $thresholdmax. It also changes the query to use SQL with parameters for authority types. Test plan : 1) On a catalog create a new authority 2) Be sure catalog is well indexed 3) Run misc/migration_tools/remove_unused_authorities.pl -t 4) You will see the line : *** Testing only, authorities will not be deleted. *** 5) You will see lines of : authid=x type=y : used X time(s) 6) You will see the line for the authority created in 1) : authid=x type=y : can be deleted 7) You will see at the end : x authorities parsed y can be deleted because unused z unchanged because used 8) Run misc/migration_tools/remove_unused_authorities.pl 9) You don't see the line : *** Testing only, authorities will not be deleted. *** 10) You will see lines of : authid=x type=y : used X time(s) 11) You will see the line for the authority created in 1) : authid=x type=y : deleted 12) You will see at the end : x authorities parsed y deleted because unused z unchanged because used 13) Run misc/migration_tools/remove_unused_authorities.pl --auth NP --auth CO 14) You see the line : Restricted to authority type(s) : NP,CO. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 102837 [details] [review] Bug 21865: ES compatiblity of remove_unused_authorities.pl The script misc/migration_tools/remove_unused_authorities.pl directly checks if Zebra search is OK. This patch changes so that this test is only if Zebra is the search engine. It also adds a test on the search off any authority number indexed (index 'an'). With Zebra its : an,alwaysmatches='' With ES its : an:* This test ensure that biblios records are indexed and that not all autorities will be deleted. Test plan: 1) On a catalog create a new authority 2) Use Zebra in systempreference SearchEngine 3) Stop Zebra server 4) Run misc/migration_tools/remove_unused_authorities.pl -c 5) The script does nothing and says : Zebra server seems not to be available. This script needs Zebra runs. 6) Restart Zebra server 7) Delete biblio index base 8) Run misc/migration_tools/remove_unused_authorities.pl -c 9) The script does nothing and says : Searching authority number in biblio records seems not to be available : an,alwaysmatches='' 10) Use ElasticSearch in systempreference SearchEngine 11) Delete biblio index base 12) Run misc/migration_tools/remove_unused_authorities.pl -c 13) The script does nothing and says : Searching authority number in biblio records seems not to be available : an:* Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 102838 [details] [review] Bug 21865: add confirm to remove_unused_authorities.pl script Like in most scripts in misc, add confirm argument to ensure script is not run without knowing what it does. Test plan: 1) Run misc/migration_tools/remove_unused_authorities.pl -h 2) You see help line for confirm 3) Run misc/migration_tools/remove_unused_authorities.pl 4) You see help and script does nothing 5) Run misc/migration_tools/remove_unused_authorities.pl -c 6) Script runs like wanted Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 102839 [details] [review] Bug 21865: allow test arg alone Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work everyone! Pushed to master for 20.05
backported to 19.11.x for 19.11.06