Move indexation-related subs from rebuild_zebra.pl to new module C4::ZebraIndex and create two new subs IndexRecord and DeleteRecordIndex This should allow easier indexation from scripts other than rebuild_zebra.pl
Created attachment 35883 [details] [review] Bug 13705: Move indexation subs in C4::ZebraIndex Move indexation-related subs from rebuild_zebra.pl to new module C4::ZebraIndex and create two new subs IndexRecord and DeleteRecordIndex This should allow easier indexation from scripts other than rebuild_zebra.pl
Created attachment 35884 [details] [review] Bug 13705: Prevent a warning in _fix_unimarc_100
To test, run rebuild_zebra.pl trying the different options. The behaviour should remain identical.
We still add things on C4? Just a question 1) License need to be updated to GPL v3 2) koha-qa complains FAIL C4/ZebraIndex.pm OK pod OK forbidden patterns OK critic FAIL valid "my" variable $string masks earlier declaration in same scope
Created attachment 40060 [details] [review] Bug 13705 - Update copyright and GPL Licence statement in C4/ZebraIndex.pm
Created attachment 40061 [details] [review] Bug 13705 - Fix QA comments
The 2 last patch are follow up fixing licence statement and a qa warning.
Hi, new errors FAIL C4/ZebraIndex.pm OK critic OK forbidden patterns OK pod OK spelling 0 FAIL valid Name "Cache::RemovalStrategy::FIELDS" used only once: possible typo Name "Cache::RemovalStrategy::LRU::FIELDS" used only once: possible typo Name "Tie::Hash::FIELDS" used only once: possible typo
I think you can continue - this should be fixed by another patch that passed QA recently.
Created attachment 49888 [details] [review] [SIGNED_OFF] Bug 13705: Move indexation subs in C4::ZebraIndex Move indexation-related subs from rebuild_zebra.pl to new module C4::ZebraIndex and create two new subs IndexRecord and DeleteRecordIndex This should allow easier indexation from scripts other than rebuild_zebra.pl Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 49889 [details] [review] [SIGNED_OFF] Bug 13705: Prevent a warning in _fix_unimarc_100 Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 49890 [details] [review] [SIGNED-OFF] Bug 13705 - Update copyright and GPL Licence statement in C4/ZebraIndex.pm Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 49891 [details] [review] [SIGNED-OFF] Bug 13705 - Fix QA comments Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Tested using options -v -a -b -x -r -w -z --length --offset -nosanitize No problems found, same results as before. Output slightly different: Old) ==================== exporting authority ==================== 301.................................................................................................... Records exported: 389 ==================== REINDEXING zebra ==================== New) ==================== REINDEXING zebra ==================== ....................................................................................................100 ....................................................................................................200 ....................................................................................................300 ......................................................................................... 389 records exported to /tmp/PBlyusIn1Z/exported_records ==================== REINDEXING zebra =================== No errors save for those on comment #8
I would be great to see some tests to cover this new module (even if it's not new code).
Just a general question: why C4 and not Koha?
(In reply to Katrin Fischer from comment #15) > Just a general question: why C4 and not Koha? The first version of this patch was written in 2012 (for bug 7419), that's why :) Should we move the code to Koha::ZebraIndex ?
(In reply to Jonathan Druart from comment #14) > I would be great to see some tests to cover this new module (even if it's > not new code). How can we test code that should modify the Zebra index, without really modifying it ? Should we create another Zebra instance for tests purpose ?
(In reply to Julian Maurice from comment #17) > (In reply to Jonathan Druart from comment #14) > > I would be great to see some tests to cover this new module (even if it's > > not new code). > How can we test code that should modify the Zebra index, without really > modifying it ? Should we create another Zebra instance for tests purpose ? We already do that in t/db_dependent/www/search_utf8.t and we do not care about not modifying the Zebra index.
Looks obsolete since we have Koha/SearchEngine/Zebra
(In reply to Fridolin Somers from comment #19) > Looks obsolete since we have Koha/SearchEngine/Zebra Yeah, Koha/SearchEngine/Zebra/Indexer.pm would be the best place to house this code. "./misc/migration_tools/rebuild_zebra.pl" is nearly 1000 lines long, which is not a good idea for a CLI script. I'd say this change has merit, but... it's super old. Personally, I'd be tempted to mark this one as RESOLVED WISHLIST, as it would be nice to have, but with the gradual shift to Elasticsearch... I don't know if it's really worth the time.