[Bug 31897] New: New hook when indexing with Elasticsearch
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Bug ID: 31897 Summary: New hook when indexing with Elasticsearch Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Plugin architecture Assignee: koha-bugs@lists.koha-community.org Reporter: stefan.berndtsson@ub.gu.se QA Contact: testopia@bugs.koha-community.org This patch adds a new hook "before_index_action" with the action "update". This is run on the array reference to the biblio records that are about to be updated in the Elasticsearch index. With this, a plugin can change the record so that it better suits the index without having to change the actual record in the database (something that might be prohibited or discouraged due to centralised cataloging). -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 --- Comment #1 from Stefan Berndtsson <stefan.berndtsson@ub.gu.se> --- Created attachment 142268 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142268&action=edit Bug 31897: New before_index_action hook for Elasticsearch Hooks added: before_index_action with new action update How to test: Run tests in t/db_dependent/Koha/Plugins/Elasticsearch_Indexer_hooks.t Sponsored by: Gothenburg University Library -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Stefan Berndtsson <stefan.berndtsson@ub.gu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |stefan.berndtsson@ub.gu.se |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142268|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 142280 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142280&action=edit Bug 31897: New before_index_action hook for Elasticsearch Hooks added: before_index_action with new action update How to test: Run tests in t/db_dependent/Koha/Plugins/Elasticsearch_Indexer_hooks.t Sponsored by: Gothenburg University Library Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |m.de.rooy@rijksmuseum.nl --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- FAIL t/db_dependent/Koha/Plugins/Elasticsearch_Indexer_hooks.t OK critic FAIL file permissions File must have the exec flag Sorry, but it should not take you too much time to get those four back in the queue ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Stefan Berndtsson <stefan.berndtsson@ub.gu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142280|0 |1 is obsolete| | --- Comment #4 from Stefan Berndtsson <stefan.berndtsson@ub.gu.se> --- Created attachment 142308 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142308&action=edit Bug 31897: New before_index_action hook for Elasticsearch Hooks added: before_index_action with new action update How to test: Run tests in t/db_dependent/Koha/Plugins/Elasticsearch_Indexer_hooks.t Sponsored by: Gothenburg University Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Stefan Berndtsson <stefan.berndtsson@ub.gu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #5 from Stefan Berndtsson <stefan.berndtsson@ub.gu.se> --- Fixed file permissions and some trailing whitespace in this one as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Stefan Berndtsson from comment #5)
Fixed file permissions and some trailing whitespace in this one as well.
If you only do cosmetic fixing, keep the previous signoff line and change status back to Signed off. No need to wait for another signoff imo. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142308|0 |1 is obsolete| | --- Comment #7 from David Nind <david@davidnind.com> --- Created attachment 142441 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142441&action=edit Bug 31897: New before_index_action hook for Elasticsearch Hooks added: before_index_action with new action update How to test: Run tests in t/db_dependent/Koha/Plugins/Elasticsearch_Indexer_hooks.t Sponsored by: Gothenburg University Library Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- $VAR1 = { 'cxn_pool' => 'Static', 'index_name' => 'koha_myclone', 'nodes' => [ 'localhost:9200' ] }; Result of Koha::SearchEngine::Elasticsearch->get_elasticsearch_params; You test that to determine if Elastic exists. Wrong test. Later you crash: Exception 'Koha::Exceptions::Elasticsearch::BadResponse' thrown 'Bad response received when submitting request to Elasticsearch' with details => No nodes are available: [http://localhost:9200], type => NoNodes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- $schema->storage->txn_rollback; Koha::Plugins::Methods->delete; Really, this is wrong. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- This still needs attention. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |31734 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31734 [Bug 31734] Hooks for account, recall, indexing and more actions for holds. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 --- Comment #11 from Stefan Berndtsson <stefan.berndtsson@ub.gu.se> --- (In reply to Marcel de Rooy from comment #8)
$VAR1 = { 'cxn_pool' => 'Static', 'index_name' => 'koha_myclone', 'nodes' => [ 'localhost:9200' ] };
Result of Koha::SearchEngine::Elasticsearch->get_elasticsearch_params; You test that to determine if Elastic exists. Wrong test.
Later you crash: Exception 'Koha::Exceptions::Elasticsearch::BadResponse' thrown 'Bad response received when submitting request to Elasticsearch' with details => No nodes are available: [http://localhost:9200], type => NoNodes
I'm really not sure what I'm supposed to do here. This is based on what the test t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t does, with the Plugin parts added to it. That test does the same result-check of get_elasticsearch_params, and if there is no elasticsearch running (localhost:9200 or equivalent) it also fails with the NoNodes error. I can't see how I can come up with a solution to a test depending on a running Elasticsearch without having a running Elasticsearch. The current test does not like it when there is an Elastic running but Zebra is set as the engine, so it'll have to be tweaked (the existing Indexer.t works here). I did some more copy&paste from that one so I can make this test work as well regardless of that syspref, but not without actually having Elastic running. Suggestions are welcome. :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Stefan Berndtsson from comment #11)
I'm really not sure what I'm supposed to do here. This is based on what the test t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t does, with the Plugin parts added to it.
That test does the same result-check of get_elasticsearch_params, and if there is no elasticsearch running (localhost:9200 or equivalent) it also fails with the NoNodes error.
I can't see how I can come up with a solution to a test depending on a running Elasticsearch without having a running Elasticsearch.
The current test does not like it when there is an Elastic running but Zebra is set as the engine, so it'll have to be tweaked (the existing Indexer.t works here). I did some more copy&paste from that one so I can make this test work as well regardless of that syspref, but not without actually having Elastic running.
I would fail Indexer.t now too for the same reason indeed. But it is pushed already ;) I am not an Elastic expert. Please ask on IRC or dev list. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 --- Comment #13 from Stefan Berndtsson <stefan.berndtsson@ub.gu.se> --- (In reply to Marcel de Rooy from comment #12)
I would fail Indexer.t now too for the same reason indeed. But it is pushed already ;) I am not an Elastic expert. Please ask on IRC or dev list.
I'll have to put this on hold for the time being. It's not within the scope of this patch, nor unfortunately within the scope of my available time and resources to deal with a solution to the generic Elasticsearch testing model. We're at the end of a release-phase now, and the actual code in this works for us. It'll have to be resumed at some later date. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #14 from Fridolin Somers <fridolin.somers@biblibre.com> --- This looks great -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 --- Comment #15 from Fridolin Somers <fridolin.somers@biblibre.com> --- Ah we should give to the plugin the index name 'biblios' or 'authorities'. Maybe with $self->index.
a plugin can change the record so that it better suits the index In this case we may better hook in Koha::SearchEngine::Elasticsearch::marc_records_to_documents(). There already is feature for ISBN forms and IncludeSeeFromInSearches.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #16 from Frédéric Demians <frederic@tamil.fr> --- See also bug 36433 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142441|0 |1 is obsolete| | --- Comment #17 from David Gustafsson <glasklas@gmail.com> --- Created attachment 178283 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178283&action=edit Bug 31897: New before_index_action hook for Elasticsearch Hooks added: before_index_action with new action update How to test: Run tests in t/db_dependent/Koha/Plugins/Elasticsearch_Indexer_hooks.t Sponsored by: Gothenburg University Library Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff CC| |glasklas@gmail.com --- Comment #18 from David Gustafsson <glasklas@gmail.com> --- (In reply to Fridolin Somers from comment #15)
Ah we should give to the plugin the index name 'biblios' or 'authorities'. Maybe with $self->index.
a plugin can change the record so that it better suits the index In this case we may better hook in Koha::SearchEngine::Elasticsearch::marc_records_to_documents(). There already is feature for ISBN forms and IncludeSeeFromInSearches.
I added the index parameter. I'm a little bit confused with regard to why marc_records_to_documents would be a better place to place the hook. In bug 36433 the hook is called when the mappings has already been processed, it seams more useful to be able to modify the record before that if we are performing some kind of cleanup/normalization. bug 36433 fits a whole different use-case if you want to perform some manual custom mapping for example, if I'm understanding things correctly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Roman Dolny <roman.dolny@jezuici.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |roman.dolny@jezuici.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Michaela Sieber <michaela.sieber@kit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36433 CC| |michaela.sieber@kit.edu -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 --- Comment #19 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to David Gustafsson from comment #18) OK, thanks for the info -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Michaela Sieber <michaela.sieber@kit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=39552 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178283|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 --- Comment #20 from Hammat wele <hammat.wele@inlibro.com> --- Created attachment 200987 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200987&action=edit Bug 31897: New before_index_action hook for Elasticsearch Hooks added: before_index_action with new action update How to test: Run tests in t/db_dependent/Koha/Plugins/Elasticsearch_Indexer_hooks.t Sponsored by: Gothenburg University Library Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31897 Hammat wele <hammat.wele@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hammat.wele@inlibro.com Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org