Bug 21865 - Add Elasticsearch support to, and improve verbose output of, `remove_unused_authorities.pl`
Summary: Add Elasticsearch support to, and improve verbose output of, `remove_unused_a...
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Fridolin SOMERS
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-20 09:38 UTC by Fridolin SOMERS
Modified: 2020-05-18 21:40 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06


Attachments
Bug 21865: improve remove_unused_authorities.pl script (5.18 KB, patch)
2018-11-21 08:17 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 21865: ES compatiblity of remove_unused_authorities.pl (3.31 KB, patch)
2018-11-21 08:18 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 21865: add confirm to remove_unused_authorities.pl script (2.10 KB, patch)
2018-11-21 08:18 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 21865: allow test arg alone (788 bytes, patch)
2020-03-11 14:46 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 21865: improve remove_unused_authorities.pl script (5.23 KB, patch)
2020-03-17 17:15 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 21865: ES compatiblity of remove_unused_authorities.pl (3.37 KB, patch)
2020-03-17 17:15 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 21865: add confirm to remove_unused_authorities.pl script (2.17 KB, patch)
2020-03-17 17:15 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 21865: allow test arg alone (851 bytes, patch)
2020-03-17 17:15 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 21865: improve remove_unused_authorities.pl script (5.30 KB, patch)
2020-04-13 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21865: ES compatiblity of remove_unused_authorities.pl (3.44 KB, patch)
2020-04-13 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21865: add confirm to remove_unused_authorities.pl script (2.24 KB, patch)
2020-04-13 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21865: allow test arg alone (925 bytes, patch)
2020-04-13 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin SOMERS 2018-11-20 09:38:56 UTC
remove_unused_authorities.pl script could be improved.
At least : test mode more verbose, ES compatibility, add confirm arg
Comment 1 Fridolin SOMERS 2018-11-21 08:17:56 UTC
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.
Comment 2 Fridolin SOMERS 2018-11-21 08:18:11 UTC
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:*
Comment 3 Fridolin SOMERS 2018-11-21 08:18:51 UTC
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
Comment 4 Bernardo Gonzalez Kriegel 2020-02-28 16:51:25 UTC
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)
Comment 5 Fridolin SOMERS 2020-03-11 14:35:20 UTC
(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
Comment 6 Fridolin SOMERS 2020-03-11 14:46:46 UTC
Created attachment 100577 [details] [review]
Bug 21865: allow test arg alone
Comment 7 Bernardo Gonzalez Kriegel 2020-03-17 17:15:41 UTC
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>
Comment 8 Bernardo Gonzalez Kriegel 2020-03-17 17:15:44 UTC
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>
Comment 9 Bernardo Gonzalez Kriegel 2020-03-17 17:15:48 UTC
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>
Comment 10 Bernardo Gonzalez Kriegel 2020-03-17 17:15:51 UTC
Created attachment 100921 [details] [review]
Bug 21865: allow test arg alone

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment 11 Bernardo Gonzalez Kriegel 2020-03-17 17:16:34 UTC
Tested with zebra, no errors.
Comment 12 Bernardo Gonzalez Kriegel 2020-03-18 13:54:53 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #11)
> Tested with zebra, no errors.

And tested with Elasticsearch, no errors
Comment 13 Jonathan Druart 2020-04-13 13:45:21 UTC
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>
Comment 14 Jonathan Druart 2020-04-13 13:45:25 UTC
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>
Comment 15 Jonathan Druart 2020-04-13 13:45:28 UTC
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>
Comment 16 Jonathan Druart 2020-04-13 13:45:31 UTC
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>
Comment 17 Martin Renvoize 2020-04-14 16:23:10 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 18 Joy Nelson 2020-05-05 22:08:14 UTC
backported to 19.11.x for 19.11.06