Bug 13705 - Move indexation subs in C4::ZebraIndex
Summary: Move indexation subs in C4::ZebraIndex
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Alex Arnaud
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 13706
  Show dependency treegraph
 
Reported: 2015-02-13 08:32 UTC by Julian Maurice
Modified: 2016-07-22 06:11 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 13705: Move indexation subs in C4::ZebraIndex (36.74 KB, patch)
2015-02-13 08:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13705: Prevent a warning in _fix_unimarc_100 (1.26 KB, patch)
2015-02-13 08:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13705 - Update copyright and GPL Licence statement in C4/ZebraIndex.pm (2.00 KB, patch)
2015-06-10 12:44 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13705 - Fix QA comments (722 bytes, patch)
2015-06-10 12:44 UTC, Alex Arnaud
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 13705: Move indexation subs in C4::ZebraIndex (36.82 KB, patch)
2016-04-05 01:44 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 13705: Prevent a warning in _fix_unimarc_100 (1.33 KB, patch)
2016-04-05 01:44 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13705 - Update copyright and GPL Licence statement in C4/ZebraIndex.pm (2.07 KB, patch)
2016-04-05 01:44 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13705 - Fix QA comments (1.77 KB, patch)
2016-04-05 01:44 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2015-02-13 08:32:41 UTC
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
Comment 1 Julian Maurice 2015-02-13 08:33:43 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2015-02-13 08:33:46 UTC Comment hidden (obsolete)
Comment 3 Julian Maurice 2015-02-13 08:35:16 UTC
To test, run rebuild_zebra.pl trying the different options. The behaviour should remain identical.
Comment 4 Bernardo Gonzalez Kriegel 2015-05-02 01:38:51 UTC
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
Comment 5 Alex Arnaud 2015-06-10 12:44:55 UTC Comment hidden (obsolete)
Comment 6 Alex Arnaud 2015-06-10 12:44:59 UTC Comment hidden (obsolete)
Comment 7 Alex Arnaud 2015-06-10 12:48:05 UTC
The 2 last patch are follow up fixing licence statement and a qa warning.
Comment 8 Bernardo Gonzalez Kriegel 2016-04-04 19:04:11 UTC
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
Comment 9 Katrin Fischer 2016-04-04 20:39:36 UTC
I think you can continue - this should be fixed by another patch that passed QA recently.
Comment 10 Bernardo Gonzalez Kriegel 2016-04-05 01:44:16 UTC
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>
Comment 11 Bernardo Gonzalez Kriegel 2016-04-05 01:44:24 UTC
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>
Comment 12 Bernardo Gonzalez Kriegel 2016-04-05 01:44:31 UTC
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>
Comment 13 Bernardo Gonzalez Kriegel 2016-04-05 01:44:40 UTC
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
Comment 14 Jonathan Druart 2016-04-14 06:57:31 UTC
I would be great to see some tests to cover this new module (even if it's not new code).
Comment 15 Katrin Fischer 2016-04-14 09:38:57 UTC
Just a general question: why C4 and not Koha?
Comment 16 Julian Maurice 2016-04-14 10:10:13 UTC
(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 ?
Comment 17 Julian Maurice 2016-04-14 10:18:56 UTC
(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 ?
Comment 18 Jonathan Druart 2016-04-15 07:17:51 UTC
(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.