Bug 20073 - Move Elasticsearch settings to configuration files
Summary: Move Elasticsearch settings to configuration files
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Ere Maijala
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 20390 19604 19893 20244
  Show dependency treegraph
 
Reported: 2018-01-23 13:17 UTC by Ere Maijala
Modified: 2019-03-29 11:13 UTC (History)
10 users (show)

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


Attachments
Bug 20073: Move Elasticsearch configs to yaml files and improve the default settings. (27.14 KB, patch)
2018-01-23 13:22 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 20073: Move Elasticsearch configs to yaml files and improve the default settings. (27.14 KB, patch)
2018-01-23 13:49 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 20073: Add database and mappings UI support for isbn and stdno field types. (4.05 KB, patch)
2018-01-23 13:49 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings. (24.80 KB, patch)
2018-03-16 15:38 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings. (24.89 KB, patch)
2018-03-21 12:58 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings. (24.95 KB, patch)
2018-03-21 16:12 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings. (24.63 KB, patch)
2018-03-28 12:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings. (24.00 KB, patch)
2018-04-18 10:54 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings. (23.36 KB, patch)
2018-04-18 11:01 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 20073: Remove perlcritic error (1.26 KB, patch)
2018-04-23 18:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20073: (follow-up) Correct paths (3.58 KB, patch)
2018-05-18 15:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20073: Move Elasticsearch yaml files back to admin directory (3.17 KB, patch)
2018-05-21 07:53 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings. (23.49 KB, patch)
2018-06-05 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20073: Remove perlcritic error (1.33 KB, patch)
2018-06-05 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20073: Move Elasticsearch yaml files back to admin directory (3.24 KB, patch)
2018-06-05 18:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20073: (RM follow-up) Remove atomicupdate (2.65 KB, patch)
2018-06-22 22:02 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20073: (RM follow-up) Update schema files (1.58 KB, patch)
2018-06-27 14:33 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 Ere Maijala 2018-01-23 13:17:01 UTC
There are a lot of settings in the code making customization difficult. These need to be moved to configuration files that can be overridden as necessary.
Comment 1 Ere Maijala 2018-01-23 13:22:59 UTC
Created attachment 70827 [details] [review]
Bug 20073: Move Elasticsearch configs to yaml files and improve the default settings.
Comment 2 Ere Maijala 2018-01-23 13:29:50 UTC
The patch contains the following changes:
- Index settings moved from code to etc/searchengine/elasticsearch/index_config.yaml. An alternative can be specified in koha-conf.xml.
- Field settings moved from code to etc/searchengine/elasticsearch/field_config.yaml. An alternative can be specified in koha-conf.xml.
- Default settings have been improved to remove punctuation from phrases used for sorting etc.
- State variables are used for storing configuration to avoid parsing it multiple times.
- A possibility to reset the fields too has been added to the reset operation of mappings administration.
- mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch.
- An stdno field type has been added for standard identifiers.
Comment 3 Ere Maijala 2018-01-23 13:37:39 UTC
Comment on attachment 70827 [details] [review]
Bug 20073: Move Elasticsearch configs to yaml files and improve the default settings.

Oops, I omitted the required DB and UI changes.
Comment 4 Ere Maijala 2018-01-23 13:49:19 UTC
Created attachment 70836 [details] [review]
Bug 20073: Move Elasticsearch configs to yaml files and improve the default settings.
Comment 5 Ere Maijala 2018-01-23 13:49:22 UTC
Created attachment 70837 [details] [review]
Bug 20073: Add database and mappings UI support for isbn and stdno field types.
Comment 6 Ere Maijala 2018-01-23 13:52:03 UTC
Ok, so I'm fumbling a bit with git-bz here, but the current patches should do it.  Note that the isbn field type has been added in anticipation of a future patch that adds a bit of special handling for ISBNs so that they're indexed in all forms.
Comment 7 Nicolas Legrand 2018-03-14 09:29:44 UTC
How could I test it?
Comment 8 Nick Clemens 2018-03-16 09:33:06 UTC
There are some stray sections related to payment systems left in the /etc/koha-conf.xml file and some whitespace errors.


This seems to work otherwise, need a test plan, it might be easier to separate the move to files and the updates to mappings to two separate bugs for simpler tests plans, or can be comprehensive here.

Should this depend on the improvement to mappings reset provided else where?
Comment 9 Ere Maijala 2018-03-16 10:10:56 UTC
Thanks for the comments. I'll work on adding a test plan and rebasing the patches. This doesn't necessarily require Bug 20248, though testing is more difficult without it.
Comment 10 Ere Maijala 2018-03-16 15:38:11 UTC
Created attachment 73054 [details] [review]
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings.

Improvements:
1) Index settings moved from code to etc/searchengine/elasticsearch/index_config.yaml. An alternative can be specified in koha-conf.xml.
2) Field settings moved from code to etc/searchengine/elasticsearch/field_config.yaml. An alternative can be specified in koha-conf.xml.
3) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. An alternative can be specified in koha-conf.xml.
4) Default settings have been improved to remove punctuation from phrases used for sorting etc.
5) State variables are used for storing configuration to avoid parsing it multiple times.
6) A possibility to reset the fields too has been added to the reset operation of mappings administration.
7) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch.
8) An stdno field type has been added for standard identifiers.

To test:
1) Run tests in t/Koha/SearchEngine/Elasticsearch.t
2) Clear tables search_fields and search_marc_map
3) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
4) Verify that admin/searchengine/elasticsearch/mappings.pl displays the mappings properly, including ISBN and other standard number fields.
5) Index some records
6) Verify that you can find the records
7) Put <elasticsearch_index_mappings>non_existent</elasticsearch_index_mappings> to koha-conf.xml
8) Verify that admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 fails because it can't find non_existent.
9) Copy etc/searchengine/elasticsearch/mappings.yaml to a new location and make elasticsearch_index_mappings setting in koha-conf.xml point to it.
10) Make a change in the new mappings.yaml.
11) Clear table search_fields (mappings reset doesn't do it yet, see bug 20248)
12) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
13) Verify that the changes you made are now visible in the mappings UI
Comment 11 Nicolas Legrand 2018-03-19 12:49:31 UTC
Hey Ere,

my install (Unimarc, ES 5.2.8) failed at step 6.

I can index and find books on master. With 20073 applied The index seems to work, I don't have deprecated messages as on master. But I can't find anything, my index is empty:

$ curl http://localhost:9200/_cat/indices?v
health status index                  uuid                   pri rep docs.count docs.deleted store.size pri.store.size
yellow open   koha_robin_biblios     oDy7ppLWTkS4uOeta1KijA   5   1          0            0       810b           810b
yellow open   koha_robin_authorities TFRTgnFYTCusa9pTuTI35Q   5   1       3865            0      6.9mb          6.9mb

On master, it is filled as it should:

$ curl http://localhost:9200/_cat/indices?v
health status index                  uuid                   pri rep docs.count docs.deleted store.size pri.store.size
yellow open   koha_robin_authorities 0vd2mF0aTWCyvsKtQlrkhw   5   1       3865            0      4.9mb          4.9mb
yellow open   koha_robin_biblios     hto7dbqNSSSN3Y4IG0FK0Q   5   1        856            0       10mb           10mb

Or maybe I am missing something?
Comment 12 Ere Maijala 2018-03-19 13:44:19 UTC
Hi Nicolas,

The test step 5 was probably quite bad. You should make sure to run misc/search_tools/rebuild_elastic_search.pl long enough for it to commit at least one set of records before it creates the index with the correct settings. Otherwise indexing will quietly fail (this would be the subject of another bug). So, something like

misc/search_tools/rebuild_elastic_search.pl -d -b -v -v

should do the trick if you make sure that you don't try to save any records in the cataloguing UI before the first commit. Otherwise you'll probably find only errors in /var/log/elasticsearch/elasticsearch.log. Bug 20248 contains some improvements to the indexing script.
Comment 13 Nicolas Legrand 2018-03-19 15:00:39 UTC
I have a lot of:

[2018-03-19T15:55:21,033][DEBUG][o.e.a.b.TransportShardBulkAction] [0aw4dLe] [koha_robin_biblios][2] failed to execute bulk item (index) BulkShardRequest [[ko
ha_robin_biblios][2]] containing [21] requests

I'm also on a dev install and not on a kohadevbox. I try to make and make install again, to take in account the etc changes, but it didn't do any good... Is is working for you?
Comment 14 Ere Maijala 2018-03-19 15:49:02 UTC
We did a bit of testing but couldn't quite figure the issue out yet. It's working for me with MARC 21 and Unimarc. If anyone else is up to it, please feel free to test and provide feedback.
Comment 15 Ere Maijala 2018-03-19 16:04:04 UTC
Nicolas kindly provided me with test set of Unimarc records. I was able to import and index them without issues. Perhaps the issue something specific to ES version or something. I'm running 5.6.8.
Comment 16 Ere Maijala 2018-03-21 12:58:16 UTC
Created attachment 73138 [details] [review]
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings.

Improvements:
1) Index settings moved from code to etc/searchengine/elasticsearch/index_config.yaml. An alternative can be specified in koha-conf.xml.
2) Field settings moved from code to etc/searchengine/elasticsearch/field_config.yaml. An alternative can be specified in koha-conf.xml.
3) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. An alternative can be specified in koha-conf.xml.
4) Default settings have been improved to remove punctuation from phrases used for sorting etc.
5) State variables are used for storing configuration to avoid parsing it multiple times.
6) A possibility to reset the fields too has been added to the reset operation of mappings administration.
7) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch.
8) An stdno field type has been added for standard identifiers.

To test:
1) Run tests in t/Koha/SearchEngine/Elasticsearch.t
2) Clear tables search_fields and search_marc_map
3) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
4) Verify that admin/searchengine/elasticsearch/mappings.pl displays the mappings properly, including ISBN and other standard number fields.
5) Index some records using the -d parameter with misc/search_tools/rebuild_elastic_search.pl to recreate the index
6) Verify that you can find the records
7) Put <elasticsearch_index_mappings>non_existent</elasticsearch_index_mappings> to koha-conf.xml
8) Verify that admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 fails because it can't find non_existent.
9) Copy etc/searchengine/elasticsearch/mappings.yaml to a new location and make elasticsearch_index_mappings setting in koha-conf.xml point to it.
10) Make a change in the new mappings.yaml.
11) Clear table search_fields (mappings reset doesn't do it yet, see bug 20248)
12) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
13) Verify that the changes you made are now visible in the mappings UI
Comment 17 Ere Maijala 2018-03-21 12:59:20 UTC
I managed to reproduce the problem and found a stupid typo in field_config.yaml. The latest patch should work.
Comment 18 Nicolas Legrand 2018-03-21 16:12:45 UTC
Created attachment 73144 [details] [review]
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings.

Improvements:
1) Index settings moved from code to etc/searchengine/elasticsearch/index_config.yaml. An alternative can be specified in koha-conf.xml.
2) Field settings moved from code to etc/searchengine/elasticsearch/field_config.yaml. An alternative can be specified in koha-conf.xml.
3) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. An alternative can be specified in koha-conf.xml.
4) Default settings have been improved to remove punctuation from phrases used for sorting etc.
5) State variables are used for storing configuration to avoid parsing it multiple times.
6) A possibility to reset the fields too has been added to the reset operation of mappings administration.
7) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch.
8) An stdno field type has been added for standard identifiers.

To test:
1) Run tests in t/Koha/SearchEngine/Elasticsearch.t
2) Clear tables search_fields and search_marc_map
3) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
4) Verify that admin/searchengine/elasticsearch/mappings.pl displays the mappings properly, including ISBN and other standard number fields.
5) Index some records using the -d parameter with misc/search_tools/rebuild_elastic_search.pl to recreate the index
6) Verify that you can find the records
7) Put <elasticsearch_index_mappings>non_existent</elasticsearch_index_mappings> to koha-conf.xml
8) Verify that admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 fails because it can't find non_existent.
9) Copy etc/searchengine/elasticsearch/mappings.yaml to a new location and make elasticsearch_index_mappings setting in koha-conf.xml point to it.
10) Make a change in the new mappings.yaml.
11) Clear table search_fields (mappings reset doesn't do it yet, see bug 20248)
12) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
13) Verify that the changes you made are now visible in the mappings UI

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 19 Nick Clemens 2018-03-28 12:31:05 UTC
Created attachment 73383 [details] [review]
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings.

Improvements:
1) Index settings moved from code to etc/searchengine/elasticsearch/index_config.yaml. An alternative can be specified in koha-conf.xml.
2) Field settings moved from code to etc/searchengine/elasticsearch/field_config.yaml. An alternative can be specified in koha-conf.xml.
3) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. An alternative can be specified in koha-conf.xml.
4) Default settings have been improved to remove punctuation from phrases used for sorting etc.
5) State variables are used for storing configuration to avoid parsing it multiple times.
6) A possibility to reset the fields too has been added to the reset operation of mappings administration.
7) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch.
8) An stdno field type has been added for standard identifiers.

To test:
1) Run tests in t/Koha/SearchEngine/Elasticsearch.t
2) Clear tables search_fields and search_marc_map
3) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
4) Verify that admin/searchengine/elasticsearch/mappings.pl displays the mappings properly, including ISBN and other standard number fields.
5) Index some records using the -d parameter with misc/search_tools/rebuild_elastic_search.pl to recreate the index
6) Verify that you can find the records
7) Put <elasticsearch_index_mappings>non_existent</elasticsearch_index_mappings> to koha-conf.xml
8) Verify that admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 fails because it can't find non_existent.
9) Copy etc/searchengine/elasticsearch/mappings.yaml to a new location and make elasticsearch_index_mappings setting in koha-conf.xml point to it.
10) Make a change in the new mappings.yaml.
11) Clear table search_fields (mappings reset doesn't do it yet, see bug 20248)
12) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
13) Verify that the changes you made are now visible in the mappings UI

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Jonathan Druart 2018-03-28 19:48:24 UTC
+      by visiting /cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1&reset_fields=1.

This comment will conflict with bug 20248.
Comment 21 Ere Maijala 2018-03-29 06:41:39 UTC
True. I can address that in bug 20248 if this gets committed first.
Comment 22 Jonathan Druart 2018-04-10 17:38:33 UTC
Would not it make more sense to have these new config entries inside the existing  elasticsearch one?
Comment 23 Ere Maijala 2018-04-11 06:38:03 UTC
The existing elasticsearch entry is passed to Catmandu::Store::Elasticsearch as is, so it would require either cleaning it up to exclude stuff that's irrelevant to the store or restructuring the tag in a back-incompatible way. I'm open to both, but since Koha is not in control of what may be introduced as options in Catmandu, I thought it would be better to keep it separate from everything else.
Comment 24 Jonathan Druart 2018-04-11 15:09:55 UTC
(In reply to Ere Maijala from comment #23)
> The existing elasticsearch entry is passed to Catmandu::Store::Elasticsearch
> as is, so it would require either cleaning it up to exclude stuff that's
> irrelevant to the store or restructuring the tag in a back-incompatible way.
> I'm open to both, but since Koha is not in control of what may be introduced
> as options in Catmandu, I thought it would be better to keep it separate
> from everything else.

Is it a bug or a feature? Are we suppose to have something else than server or index_name?
If not, it sounds easy to recreate it.

Tomas, Nick, opinions?
Comment 25 Ere Maijala 2018-04-12 06:02:02 UTC
(In reply to Jonathan Druart from comment #24)
> Is it a bug or a feature? Are we suppose to have something else than server
> or index_name?
> If not, it sounds easy to recreate it.

Catmandu::Store::Elasticsearch uses Search::Elasticsearch underneath, and I believe we are supposed to able to pass on whatever it accepts (see https://metacpan.org/pod/Search::Elasticsearch). There are settings like trace_to and cxn_pool that can be useful and we shouldn't limit the settings to any currently-known set of options.
Comment 26 Nick Clemens 2018-04-16 15:13:05 UTC
(In reply to Ere Maijala from comment #25)
> (In reply to Jonathan Druart from comment #24)
> > Is it a bug or a feature? Are we suppose to have something else than server
> > or index_name?
> > If not, it sounds easy to recreate it.
> 
> Catmandu::Store::Elasticsearch uses Search::Elasticsearch underneath, and I
> believe we are supposed to able to pass on whatever it accepts (see
> https://metacpan.org/pod/Search::Elasticsearch). There are settings like
> trace_to and cxn_pool that can be useful and we shouldn't limit the settings
> to any currently-known set of options.

I think this is right, we can consider these two separate sets of options - server config and koha settings, could ultimately be organized as:
<ES_settings>
  <ES_server_settings>
    <server> etc...
  </ES_server_settings>
  <ES_koha_settings>
    <ES_index_config> etc...
  </ES_koha_settings>
</ES_settings>

but this would really just be organizational and we don't do for all zebra stuff now so I would say could be new bug report and not a blocker here
Comment 27 Ere Maijala 2018-04-18 10:54:13 UTC
Created attachment 74420 [details] [review]
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings.

Improvements:
1) Index settings moved from code to etc/searchengine/elasticsearch/index_config.yaml. An alternative can be specified in koha-conf.xml.
2) Field settings moved from code to etc/searchengine/elasticsearch/field_config.yaml. An alternative can be specified in koha-conf.xml.
3) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. An alternative can be specified in koha-conf.xml.
4) Default settings have been improved to remove punctuation from phrases used for sorting etc.
5) State variables are used for storing configuration to avoid parsing it multiple times.
6) A possibility to reset the fields too has been added to the reset operation of mappings administration.
7) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch.
8) An stdno field type has been added for standard identifiers.

To test:
1) Run tests in t/Koha/SearchEngine/Elasticsearch.t
2) Clear tables search_fields and search_marc_map
3) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
4) Verify that admin/searchengine/elasticsearch/mappings.pl displays the mappings properly, including ISBN and other standard number fields.
5) Index some records using the -d parameter with misc/search_tools/rebuild_elastic_search.pl to recreate the index
6) Verify that you can find the records
7) Put <elasticsearch_index_mappings>non_existent</elasticsearch_index_mappings> to koha-conf.xml
8) Verify that admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 fails because it can't find non_existent.
9) Copy etc/searchengine/elasticsearch/mappings.yaml to a new location and make elasticsearch_index_mappings setting in koha-conf.xml point to it.
10) Make a change in the new mappings.yaml.
11) Clear table search_fields (mappings reset doesn't do it yet, see bug 20248)
12) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
13) Verify that the changes you made are now visible in the mappings UI
Comment 28 Ere Maijala 2018-04-18 10:54:40 UTC
The new patch is same as before, just rebased on top of master.
Comment 29 Ere Maijala 2018-04-18 11:01:28 UTC
Created attachment 74421 [details] [review]
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings.

Improvements:
1) Index settings moved from code to etc/searchengine/elasticsearch/index_config.yaml. An alternative can be specified in koha-conf.xml.
2) Field settings moved from code to etc/searchengine/elasticsearch/field_config.yaml. An alternative can be specified in koha-conf.xml.
3) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. An alternative can be specified in koha-conf.xml.
4) Default settings have been improved to remove punctuation from phrases used for sorting etc.
5) State variables are used for storing configuration to avoid parsing it multiple times.
6) A possibility to reset the fields too has been added to the reset operation of mappings administration.
7) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch.
8) An stdno field type has been added for standard identifiers.

To test:
1) Run tests in t/Koha/SearchEngine/Elasticsearch.t
2) Clear tables search_fields and search_marc_map
3) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
4) Verify that admin/searchengine/elasticsearch/mappings.pl displays the mappings properly, including ISBN and other standard number fields.
5) Index some records using the -d parameter with misc/search_tools/rebuild_elastic_search.pl to recreate the index
6) Verify that you can find the records
7) Put <elasticsearch_index_mappings>non_existent</elasticsearch_index_mappings> to koha-conf.xml
8) Verify that admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 fails because it can't find non_existent.
9) Copy etc/searchengine/elasticsearch/mappings.yaml to a new location and make elasticsearch_index_mappings setting in koha-conf.xml point to it.
10) Make a change in the new mappings.yaml.
11) Clear table search_fields (mappings reset doesn't do it yet, see bug 20248)
12) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
13) Verify that the changes you made are now visible in the mappings UI
Comment 30 Ere Maijala 2018-04-18 11:23:14 UTC
Okay, this one should really be correct. The previous patch contained one unrelated change.
Comment 31 Jonathan Druart 2018-04-19 19:34:05 UTC
signed-off-by lines have been removed, do we need to retest?
Comment 32 Ere Maijala 2018-04-23 07:14:24 UTC
Should not be necessary, it was just a rebase (which I botched once, but should be fine now).
Comment 33 Jonathan Druart 2018-04-23 17:29:21 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 34 Jonathan Druart 2018-04-23 18:13:16 UTC
Created attachment 74759 [details] [review]
Bug 20073: Remove perlcritic error

"return" statement with explicit "undef" at line 245, column 5.  See
page 199 of PBP.  (Severity: 5)
Comment 35 Ere Maijala 2018-04-24 07:03:13 UTC
Thanks for the perlcritic fix, Jonathan!
Comment 36 Jonathan Druart 2018-05-18 14:49:41 UTC
There is a blocker issue introduced by these patches. The installer process (using packages) explodes with:

'/usr/share/koha/intranet/cgi-bin/etc/searchengine/elasticsearch/mappings.yaml' is empty or non-existent at /usr/lib/x86_64-linux-gnu/perl5/5.20/YAML/Syck.pm line 129.


The concatenation is obviously wrong.

the koha-conf entry is filled with:
 <!-- <elasticsearch_index_mappings>/etc/koha/sites/libraryname/searchengine/elasticsearch/mappings.yaml</elasticsearch_index_mappings> -->

Which is wrong too.

Please fix ASAP or it will be reverted before 18.05 is released.
Comment 37 Nick Clemens 2018-05-18 15:41:04 UTC Comment hidden (obsolete)
Comment 38 Ere Maijala 2018-05-21 07:29:05 UTC
The original patch moved elasticsearch.yaml from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. I didn't realize this affects packaging. I think it's best to revert the move and put the other yaml files into admin/searchengine/elasticsearch too. I'll attach a patch ASAP.
Comment 39 Ere Maijala 2018-05-21 07:53:13 UTC
Created attachment 75453 [details] [review]
Bug 20073: Move Elasticsearch yaml files back to admin directory
Comment 40 Ere Maijala 2018-05-21 07:55:48 UTC
Attached a patch that moves the default yaml files back to admin directory. Tests pass but I don't know enough about packaging to test it. The koha-conf entry is a commented-out example, and I can't see anything wrong with it.
Comment 41 Jonathan Druart 2018-05-21 13:45:16 UTC
I am going to revert this patch from master, 18.05 is released tomorrow and I will not have time to retest this one.

It seems safer to consolidate and push it later.
Comment 42 Jonathan Druart 2018-05-21 14:04:58 UTC
Reverted.
Comment 43 Jonathan Druart 2018-05-21 14:05:19 UTC
Sending back to QA queue.
Comment 44 Kyle M Hall 2018-06-05 18:20:00 UTC
Created attachment 75829 [details] [review]
Bug 20073 - Move Elasticsearch configs to yaml files and improve the default settings.

Improvements:
1) Index settings moved from code to etc/searchengine/elasticsearch/index_config.yaml. An alternative can be specified in koha-conf.xml.
2) Field settings moved from code to etc/searchengine/elasticsearch/field_config.yaml. An alternative can be specified in koha-conf.xml.
3) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. An alternative can be specified in koha-conf.xml.
4) Default settings have been improved to remove punctuation from phrases used for sorting etc.
5) State variables are used for storing configuration to avoid parsing it multiple times.
6) A possibility to reset the fields too has been added to the reset operation of mappings administration.
7) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch.
8) An stdno field type has been added for standard identifiers.

To test:
1) Run tests in t/Koha/SearchEngine/Elasticsearch.t
2) Clear tables search_fields and search_marc_map
3) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
4) Verify that admin/searchengine/elasticsearch/mappings.pl displays the mappings properly, including ISBN and other standard number fields.
5) Index some records using the -d parameter with misc/search_tools/rebuild_elastic_search.pl to recreate the index
6) Verify that you can find the records
7) Put <elasticsearch_index_mappings>non_existent</elasticsearch_index_mappings> to koha-conf.xml
8) Verify that admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 fails because it can't find non_existent.
9) Copy etc/searchengine/elasticsearch/mappings.yaml to a new location and make elasticsearch_index_mappings setting in koha-conf.xml point to it.
10) Make a change in the new mappings.yaml.
11) Clear table search_fields (mappings reset doesn't do it yet, see bug 20248)
12) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
13) Verify that the changes you made are now visible in the mappings UI

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 45 Kyle M Hall 2018-06-05 18:20:09 UTC
Created attachment 75830 [details] [review]
Bug 20073: Remove perlcritic error

"return" statement with explicit "undef" at line 245, column 5.  See
page 199 of PBP.  (Severity: 5)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 46 Kyle M Hall 2018-06-05 18:20:12 UTC
Created attachment 75831 [details] [review]
Bug 20073: Move Elasticsearch yaml files back to admin directory

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 47 Ere Maijala 2018-06-21 09:10:10 UTC
Could we get this back into master soon?
Comment 48 Katrin Fischer 2018-06-22 05:40:39 UTC
There were only bug fixes pushed for a bit of time after the new release to ease backporting, but I think pushing enh will happen soon now.
Comment 49 Nick Clemens 2018-06-22 18:10:04 UTC
Awesome work all!

Pushed to aster for 18.11.x
Comment 50 Nick Clemens 2018-06-22 18:10:56 UTC
Squashed 1st and 3rd patches on push
Comment 51 Nick Clemens 2018-06-22 22:02:43 UTC
Created attachment 76315 [details] [review]
Bug 20073: (RM follow-up) Remove atomicupdate

D'oh

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 52 Martin Renvoize 2018-06-27 09:05:00 UTC
Enhancement, not back porting to 18.05.x series.
Comment 53 David Gustafsson 2018-06-27 09:32:15 UTC
Great!
Comment 54 Jonathan Druart 2018-06-27 14:07:30 UTC
Nick, DBIC schema has not been updated.
Comment 55 Nick Clemens 2018-06-27 14:33:04 UTC
Created attachment 76520 [details] [review]
Bug 20073: (RM follow-up) Update schema files