|
Description
Alex Arnaud
2019-06-25 13:39:45 UTC
Created attachment 90999 [details] [review] Bug 23204 - script for exporting Elasticsearch mappings Test plan: - launch perl misc/search_tools/export_elasticsearch_mappings.pl > /path/to/my_mappings.yaml - set koha-conf.elasticsearch_index_mappings to /path/to/my_mappings.yaml, - go to admin -> Search engine configuration, - click on "Reset mappins", - check that your search fields and mappings are as expected Created attachment 91350 [details] [review] Bug 23204 - script for exporting Elasticsearch mappings Test plan: - launch perl misc/search_tools/export_elasticsearch_mappings.pl > /path/to/my_mappings.yaml - set koha-conf.elasticsearch_index_mappings to /path/to/my_mappings.yaml, - go to admin -> Search engine configuration, - click on "Reset mappins", - check that your search fields and mappings are as expected Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com> This is great, few things: 1 - This should be a subroutine that can be unit tested (also then we can easily integrate into staff client and allow users to export mappings) 2 - This should take preserves weights specified in the client Created attachment 92022 [details] [review] Bug 23204 - script for exporting Elasticsearch mappings Test plan: - launch perl misc/search_tools/export_elasticsearch_mappings.pl > /path/to/my_mappings.yaml - set koha-conf.elasticsearch_index_mappings to /path/to/my_mappings.yaml, - go to admin -> Search engine configuration, - click on "Reset mappins", - check that your search fields and mappings are as expected Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com> Created attachment 92023 [details] [review] Bug 23204: move code in a unit tested sub Created attachment 92024 [details] [review] Bug 23204: export field weights misc/search_tools/export_elasticsearch_mappings.pl is missing exec permissions : chmod a+x Created attachment 92174 [details] [review] Bug 23204: add exec permission on the script Really cool script. Just, the generated YAML does not ensure sorting order. Fields must be sorted by name and mappings by marc type (at least). Created attachment 93524 [details] [review] Bug 23204: sort search fields by name Created attachment 95986 [details] [review] Bug 23204: (follow-up) also sort by marc field Test plan : Launching script several times and see that marc field is alway ordered increasing Question: Should we only load/export the mappings for the marc_type of the instance? Including the normarc and unimarc for marc21 makes the file harder to read. Same the other way round to :-) (In reply to Nick Clemens from comment #12) > Question: Should we only load/export the mappings for the marc_type of the > instance? Including the normarc and unimarc for marc21 makes the file harder > to read. Same the other way round to :-) There is the --type arg in the script to export only one marc flavour. Created attachment 100960 [details] [review] Bug 23204: script for exporting Elasticsearch mappings Test plan: - launch perl misc/search_tools/export_elasticsearch_mappings.pl > /path/to/my_mappings.yaml - set koha-conf.elasticsearch_index_mappings to /path/to/my_mappings.yaml, - go to admin -> Search engine configuration, - click on "Reset mappins", - check that your search fields and mappings are as expected Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Fixed commit message Created attachment 100961 [details] [review] Bug 23204: move code in a unit tested sub Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Test pass Created attachment 100962 [details] [review] Bug 23204: export field weights Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Created attachment 100963 [details] [review] Bug 23204: add exec permission on the script Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Created attachment 100964 [details] [review] Bug 23204: sort search fields by name Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Created attachment 100965 [details] [review] Bug 23204: (follow-up) also sort by marc field Test plan : Launching script several times and see that marc field is alway ordered increasing Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Same order between runs I think the exported file must export exactly the same file as the one we embed with Koha. % perl misc/search_tools/export_elasticsearch_mappings.pl > /tmp/mappings.yaml % diff admin/searchengine/elasticsearch/mappings.yaml /tmp/mappings.yaml Should return no change. Don't you think? Created attachment 101742 [details] [review] Bug 23204: Fix ordering of fields in exported file (In reply to Jonathan Druart from comment #20) > I think the exported file must export exactly the same file as the one we > embed with Koha. > > % perl misc/search_tools/export_elasticsearch_mappings.pl > > /tmp/mappings.yaml > % diff admin/searchengine/elasticsearch/mappings.yaml /tmp/mappings.yaml > Should return no change. > > Don't you think? Yes and no - ordering fixed most, and not adding undefined weights or facet_order, but there are some remaining differences: 1 - Facet order not set in default file 2 - LC-card-number - comes out in lower case because we have it twice, once for authorities and once for biblios - these get combined when adding to DB because they all go in to 'search_field' which doesn't separate bibs/auths - I don't think the matters. There does seem to be convention of Auth fields being capitalized and bibs fields not, but I don't believe we rely on this or need to 3 - Default mapping file is not strictly alphabetized - date-of-publication moves on export Probably a good compromise is to generate the mapping file from this bug once it passes QA and use the exported file as the default from now on. Consider it a cleanup of the file (In reply to Nick Clemens from comment #22) > > Probably a good compromise is to generate the mapping file from this bug > once it passes QA and use the exported file as the default from now on. > Consider it a cleanup of the file +1 Created attachment 101850 [details] [review] Bug 23204: script for exporting Elasticsearch mappings Test plan: - launch perl misc/search_tools/export_elasticsearch_mappings.pl > /path/to/my_mappings.yaml - set koha-conf.elasticsearch_index_mappings to /path/to/my_mappings.yaml, - go to admin -> Search engine configuration, - click on "Reset mappins", - check that your search fields and mappings are as expected Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Fixed commit message Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 101851 [details] [review] Bug 23204: move code in a unit tested sub Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Test pass Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 101852 [details] [review] Bug 23204: export field weights Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 101853 [details] [review] Bug 23204: add exec permission on the script Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 101854 [details] [review] Bug 23204: sort search fields by name Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 101855 [details] [review] Bug 23204: (follow-up) also sort by marc field Test plan : Launching script several times and see that marc field is alway ordered increasing Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Same order between runs Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 101856 [details] [review] Bug 23204: Fix ordering of fields in exported file Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 101857 [details] [review] Bug 23204: Update the mappings.yaml file with the output generated We need to be consistent and provide the mappings.yaml file generated by the script. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 101858 [details] [review] Bug 23204: Adjust copyright and license Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> (In reply to Jonathan Druart from comment #31) > Created attachment 101857 [details] [review] [review] > Bug 23204: Update the mappings.yaml file with the output generated > > We need to be consistent and provide the mappings.yaml file generated by > the script. > > Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Alex, Nick, can you confirm you agree with that? If so I think we need a test to make sure further changes will be put into the correct place in the file. >
> Alex, Nick, can you confirm you agree with that?
>
> If so I think we need a test to make sure further changes will be put into
> the correct place in the file.
I agree !
(In reply to Jonathan Druart from comment #33) > (In reply to Jonathan Druart from comment #31) > > Created attachment 101857 [details] [review] [review] [review] > > Bug 23204: Update the mappings.yaml file with the output generated > > > > We need to be consistent and provide the mappings.yaml file generated by > > the script. > > > > Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> > > Alex, Nick, can you confirm you agree with that? > > If so I think we need a test to make sure further changes will be put into > the correct place in the file. See bug 24997. Nice work everyone! Pushed to master for 20.05 We would really like this one in 19.11.x Created attachment 102010 [details] [review] Bug 23204: Fix tests swapping fields (they are not ordered) enhancement not backported to 19.11.x Note : Since bug 20589, two fields have been added to 'search_field' table: staff_client and opac. Those fields are lacking in mappings.yaml. Its means that if this script is used to export and import (Reset mapping in Search engine configuration) indexing rules, the searchability (staff/opac) of fields will be lost. (In reply to Frédéric Demians from comment #40) > Note : Since bug 20589, two fields have been added to 'search_field' table: > staff_client and opac. Those fields are lacking in mappings.yaml. Its means > that if this script is used to export and import (Reset mapping in Search > engine configuration) indexing rules, the searchability (staff/opac) of > fields will be lost. I've opened a new bug report: Bug 25337 - Export/import of Elasticsearch configuration is lacking information (In reply to Jonathan Druart from comment #31) > Created attachment 101857 [details] [review] [review] > Bug 23204: Update the mappings.yaml file with the output generated > > We need to be consistent and provide the mappings.yaml file generated by > the script. > > Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> I think i agreed a little to fast this commit. Removing quote on field number starting with 0 (030) make that number is badly interpreted (as octal). So this mapping: coden: label: coden mappings: - facet: '' marc_field: 030 marc_type: marc21 sort: ~ suggestible: '' makes coden mapped with field 24 Indexing fail: root@2dc1f5ace3f5:koha(master)$ perl misc/search_tools/rebuild_elasticsearch.pl -b -a -d -v [1324] Checking state of biblios index [1324] Dropping and recreating biblios index [1324] Checking state of authorities index [1324] Dropping and recreating authorities index [1324] Indexing biblios [1324] Committing final records... Invalid MARC field expression: 24 (In reply to Alex Arnaud from comment #42) > (In reply to Jonathan Druart from comment #31) > I think i agreed a little to fast this commit. Removing quote on field > number starting with 0 (030) make that number is badly interpreted (as > octal). I experienced this once, however, I cannot recreate it now, what is your setup when this happens? This issue may depend on the YAML parser used by default. I can't neither reproduce it on my setup. Replacing "use YAML" with "use YAML::Syck" could be a solution. (In reply to Nick Clemens from comment #43) > (In reply to Alex Arnaud from comment #42) > > I experienced this once, however, I cannot recreate it now, what is your > setup when this happens? Now i can reproduce it almost each time: - start a fresh koha container (koha-testing-docker): ku-es5 or reset_all in an existing one, - Change searchengine syspref to "Elasticsearch", - Go to Admin -> Search engine configuration (Elasticsearch), - mapping ok (coden => 030, code-institution => 040 etc...) - reset mapping - mapping Ko (coden => 24, code-institution => 32 etc...) Could someone deal with that? At least to open a new bug report. I don't recreate with KOHA_IMAGE=master-bionic and docker-compose.es6.yml If there is a new bug, assign it to me. I will do my best to reproduce it and fix it. |