Bug 27348 - Error defining INDEXER_PARAMS in /etc/default/koha-common
Summary: Error defining INDEXER_PARAMS in /etc/default/koha-common
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Zebra (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: David Cook
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-06 01:37 UTC by David Cook
Modified: 2021-08-12 20:30 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.02,20.11.09


Attachments
Bug 27348: Fix test on INDEXER_PARAMS in koha-indexer (1.39 KB, patch)
2021-01-07 00:23 UTC, David Cook
Details | Diff | Splinter Review
Bug 27348: Fix test on ALTERNATE_INDEXER_DAEMON in koha-indexer (842 bytes, patch)
2021-01-07 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27348: Fix test on INDEXER_PARAMS in koha-indexer (1.44 KB, patch)
2021-06-03 23:39 UTC, David Nind
Details | Diff | Splinter Review
Bug 27348: Fix test on ALTERNATE_INDEXER_DAEMON in koha-indexer (890 bytes, patch)
2021-06-03 23:39 UTC, David Nind
Details | Diff | Splinter Review
Bug 27348: Fix test on INDEXER_PARAMS in koha-indexer (1.49 KB, patch)
2021-06-11 16:43 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27348: Fix test on ALTERNATE_INDEXER_DAEMON in koha-indexer (947 bytes, patch)
2021-06-11 16:43 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2021-01-06 01:37:51 UTC
The test at line 220 in koha-indexer creates an error since it isn't treating $INDEXER_PARAMS as a string. 

For example:

/usr/sbin/koha-indexer: line 220: [: -sleep: binary operator expected
Comment 1 Jonathan Druart 2021-01-06 10:19:14 UTC
Steps to recreate please?

In ktd:

% koha-indexer --restart kohadev
Restarting Koha indexing daemon for kohadev:.

=> it works
Comment 2 David Cook 2021-01-07 00:19:21 UTC
Steps to reproduce:

1. vi /etc/default/koha-common
2. Add the following to the bottom of the file:
INDEXER_PARAMS="-daemon -sleep $INDEXER_TIMEOUT"
3. koha-indexer --stop kohadev
4. Note the following error:
/usr/sbin/koha-indexer: line 220: [: too many arguments
Comment 3 David Cook 2021-01-07 00:23:27 UTC
Created attachment 114902 [details] [review]
Bug 27348: Fix test on INDEXER_PARAMS in koha-indexer

koha-indexer doesn't test INDEXER_PARAMS correctly which causes errors
to display when stopping/starting the daemon.

This patch fixes the test so that the variable is tested as a string,
so that no errors are created and the params are passed correctly.

Test plan:
0. Apply patch
1. vi /etc/default/koha-common
2. Add the following to the bottom of the file:
INDEXER_PARAMS="-daemon -sleep 6"
3. cp debian/scripts/koha-indexer /usr/sbin/koha-indexer
4. koha-indexer --stop kohadev
5. Note no errors
6. koha-indexer --start kohadev
7. Note no errors
8. ps -efww | grep "indexer"
9. Note that rebuild_zebra.pl has the arguments "-daemon -sleep 6"
Comment 4 Jonathan Druart 2021-01-07 09:54:14 UTC
Shouldn't we add the quote for $ALTERNATE_INDEXER_DAEMON as well?
It won't hurt :)
Comment 5 Jonathan Druart 2021-01-07 09:55:48 UTC
Created attachment 114903 [details] [review]
Bug 27348: Fix test on ALTERNATE_INDEXER_DAEMON in koha-indexer
Comment 6 Jonathan Druart 2021-01-07 09:56:54 UTC
We should also use -z for the DEV_INSTALL test

208     if [ "$DEV_INSTALL" = "" ]; then

For later...
Comment 7 David Cook 2021-01-07 22:48:56 UTC
(In reply to Jonathan Druart from comment #4)
> Shouldn't we add the quote for $ALTERNATE_INDEXER_DAEMON as well?
> It won't hurt :)

Probably wise. You never know when someone is going to put spaces in their filepath and cause problems.
Comment 8 David Nind 2021-06-03 23:39:03 UTC
Created attachment 121620 [details] [review]
Bug 27348: Fix test on INDEXER_PARAMS in koha-indexer

koha-indexer doesn't test INDEXER_PARAMS correctly which causes errors
to display when stopping/starting the daemon.

This patch fixes the test so that the variable is tested as a string,
so that no errors are created and the params are passed correctly.

Test plan:
0. Apply patch
1. vi /etc/default/koha-common
2. Add the following to the bottom of the file:
INDEXER_PARAMS="-daemon -sleep 6"
3. cp debian/scripts/koha-indexer /usr/sbin/koha-indexer
4. koha-indexer --stop kohadev
5. Note no errors
6. koha-indexer --start kohadev
7. Note no errors
8. ps -efww | grep "indexer"
9. Note that rebuild_zebra.pl has the arguments "-daemon -sleep 6"

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2021-06-03 23:39:07 UTC
Created attachment 121621 [details] [review]
Bug 27348: Fix test on ALTERNATE_INDEXER_DAEMON in koha-indexer

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Nick Clemens 2021-06-11 16:43:05 UTC
Created attachment 121861 [details] [review]
Bug 27348: Fix test on INDEXER_PARAMS in koha-indexer

koha-indexer doesn't test INDEXER_PARAMS correctly which causes errors
to display when stopping/starting the daemon.

This patch fixes the test so that the variable is tested as a string,
so that no errors are created and the params are passed correctly.

Test plan:
0. Apply patch
1. vi /etc/default/koha-common
2. Add the following to the bottom of the file:
INDEXER_PARAMS="-daemon -sleep 6"
3. cp debian/scripts/koha-indexer /usr/sbin/koha-indexer
4. koha-indexer --stop kohadev
5. Note no errors
6. koha-indexer --start kohadev
7. Note no errors
8. ps -efww | grep "indexer"
9. Note that rebuild_zebra.pl has the arguments "-daemon -sleep 6"

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens 2021-06-11 16:43:09 UTC
Created attachment 121862 [details] [review]
Bug 27348: Fix test on ALTERNATE_INDEXER_DAEMON in koha-indexer

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Jonathan Druart 2021-06-21 10:05:16 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 13 Kyle M Hall 2021-07-02 13:37:23 UTC
Pushed to 21.05.x for 21.05.02
Comment 14 Fridolin Somers 2021-08-02 19:58:43 UTC
Pushed to 20.11.x for 20.11.09
Comment 15 Victor Grousset/tuxayo 2021-08-12 20:30:56 UTC
Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed.