[Bug 28692] New: Reduce DB action_log table size
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Bug ID: 28692 Summary: Reduce DB action_log table size Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Database Assignee: koha-bugs@lists.koha-community.org Reporter: stalkernoid@gmail.com QA Contact: testopia@bugs.koha-community.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=28692 Peter Vashchuk <stalkernoid@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=28692 --- Comment #1 from Peter Vashchuk <stalkernoid@gmail.com> --- Created attachment 122683 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122683&action=edit Bug 28692 - log without indentation to decrease DB action_log table size Indentation symbols like spaces and newlines take up a large amount of extra space. Example: fully indented action_log item MODIFY record weighs from 1.6 to 370 KB while the version of it without any indentation takes 1.1 to 46 KB accordingly. This patch adds Indent(0) parameter of Data::Dumper to output without indentation. To reproduce for item: 1) edit any item you see fit, save it. 2) check the action_logs table and find there a newly created MODIFY item row, "info" column of that contains dumped data, see that it has indentation, optional: check and note about the size of it. 3) apply the patch. 4) edit that item again and save it afterwards. 5) "info" column of the newly created row should contain non-indented text now. If you did check the size of the previous dump, compare it with this new one, check the difference it makes. To reproduce for hold: 1) in syspref set HoldsLog to Log. 1) go to the biblio page and suspend and resume any hold you see fit. 2) check the action_logs table and find there a newly created SUSPEND item row, "info" column of that contains dumped data, see that it has indentation, optional: check and note about the size of it. 3) apply the patch. 4) edit that item again and save it afterwards. 5) "info" column of the newly created row should contain non-indented text now. If you did check the size of the previous dump, compare it with this new one, check the difference it makes. -- 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=28692 Peter Vashchuk <stalkernoid@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nugged@gmail.com, | |stalkernoid@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Peter Vashchuk <stalkernoid@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |stalkernoid@gmail.com |ity.org | -- 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=28692 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28700 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Yes, that makes sense. Shouldn't we actually have the Dump call in logaction and pass the Koha::Hold object? We could have something like if ( ref($infos) eq 'Koha::Hold') { $infos = Data::Dumper->new([$infos->unblessed], [qw(hold)])->Indent(0)->Sortkeys(1)->Dump; } What do you think? Don't forget to run the koha-qa.pl script before you submit a patch ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Data::Dumper->new([$self->unblessed], [qw(item)])->Indent(0)->Sortkeys(1)->Dump ) Hmm Not really like the looks of this statement personally. Not so intuitive as the easy functional interface print Dumper($a,$b) Since we use that so often, this might be confusing. Another thought: Why not set a BEGIN block in C4/Log with something like: $Data::Dumper::Indent = 0; etc And if you like preferences ;) you might wanna control this value somehow.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Peter Vashchuk <stalkernoid@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122683|0 |1 is obsolete| | --- Comment #4 from Peter Vashchuk <stalkernoid@gmail.com> --- Created attachment 122782 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122782&action=edit Bug 28692: log without indentation to decrease DB action_log table size Indentation symbols like spaces and newlines take up a large amount of extra space. Example: fully indented action_log item MODIFY record weighs from 1.6 to 370 KB while the version of it without any indentation takes 1.1 to 46 KB accordingly. This changes logaction subroutine to perform a special transformation, if$info came as known object, that adds Indent(0) parameter of Data::Dumper to output without indentation. To reproduce for item: 1) edit any item you see fit, save it. 2) check the action_logs table and find there a newly created MODIFY item row, "info" column of that contains dumped data, see that it has indentation, optional: check and note about the size of it. 3) apply the patch. 4) edit that item again and save it afterwards. 5) "info" column of the newly created row should contain non-indented text now. If you did check the size of the previous dump, compare it with this new one, check the difference it makes. To reproduce for hold: 1) in syspref set HoldsLog to Log. 1) go to the biblio page and suspend and resume any hold you see fit. 2) check the action_logs table and find there a newly created SUSPEND item row, "info" column of that contains dumped data, see that it has indentation, optional: check and note about the size of it. 3) apply the patch. 4) edit that item again and save it afterwards. 5) "info" column of the newly created row should contain non-indented text now. If you did check the size of the previous dump, compare it with this new one, check the difference it makes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |major Keywords| |release-notes-needed CC| |martin.renvoize@ptfs-europe | |.com, tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed |RM_priority -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #5 from Andrew Nugged <nugged@gmail.com> --- Note: Jonathan, Bug 28700 has much bigger impact on logs records size -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #6 from David Cook <dcook@prosentient.com.au> --- Interested in this as I recently was looking at a 30GB Koha database where 20GB was in the action_log table alone. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #7 from Fridolin Somers <fridolin.somers@biblibre.com> --- Looks great. But : Koha/Item.pm - : logaction( "CATALOGUING", "MODIFY", $self->itemnumber, "item " . Dumper( $self->unblessed ) ); + : logaction( "CATALOGUING", "MODIFY", $self->itemnumber, "item " . $self ); This is a string concatenation, seems wrong. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |joonas.kylmala@iki.fi --- Comment #8 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- (In reply to Fridolin Somers from comment #7)
Looks great.
But : Koha/Item.pm - : logaction( "CATALOGUING", "MODIFY", $self->itemnumber, "item " . Dumper( $self->unblessed ) ); + : logaction( "CATALOGUING", "MODIFY", $self->itemnumber, "item " . $self );
This is a string concatenation, seems wrong.
Yup, it's a bug. Marking as FQA to get Petro's attention. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Peter Vashchuk <stalkernoid@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Peter Vashchuk <stalkernoid@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122782|0 |1 is obsolete| | --- Comment #9 from Peter Vashchuk <stalkernoid@gmail.com> --- Created attachment 125692 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125692&action=edit Bug 28692: log without indentation to decrease DB action_log table size Indentation symbols like spaces and newlines take up a large amount of extra space. Example: fully indented action_log item MODIFY record weighs from 1.6 to 370 KB while the version of it without any indentation takes 1.1 to 46 KB accordingly. This changes logaction subroutine to perform a special transformation, if$info came as known object, that adds Indent(0) parameter of Data::Dumper to output without indentation. To reproduce for item: 1) edit any item you see fit, save it. 2) check the action_logs table and find there a newly created MODIFY item row, "info" column of that contains dumped data, see that it has indentation, optional: check and note about the size of it. 3) apply the patch. 4) edit that item again and save it afterwards. 5) "info" column of the newly created row should contain non-indented text now. If you did check the size of the previous dump, compare it with this new one, check the difference it makes. To reproduce for hold: 1) in syspref set HoldsLog to Log. 1) go to the biblio page and suspend and resume any hold you see fit. 2) check the action_logs table and find there a newly created SUSPEND item row, "info" column of that contains dumped data, see that it has indentation, optional: check and note about the size of it. 3) apply the patch. 4) edit that item again and save it afterwards. 5) "info" column of the newly created row should contain non-indented text now. If you did check the size of the previous dump, compare it with this new one, check the difference it makes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #3) Looking again here, thx for responding :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Will be testing this a bit. But can tell you now that we need tests. Might be adding one, not sure yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- I tend to think that we should solve this issue and the unblessed-thing on bug 28700 together. if ( ref $infos eq 'Koha::Hold' ) { $varname = 'hold'; } elsif ( ref $infos eq 'Koha::Item' ) { $varname = 'item'; } if ($varname) { $infos = Data::Dumper->new( [ $infos->unblessed ], [$varname] )->Indent(0)->Sortkeys(1)->Dump; } This is a bit ugly :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- The 'infos' parameter of logaction is really inconsistent in general.. I'd love to see it consistently be JSON myself.. as a universally easy thing to encode/decode with handling built-in at the object level for consistent handling of numbers and dates. I also prefer 'diff' to 'whole object dumps'.. when improving the change log for patron modification I chose to create a json representation of just the changed fields rather than dumping the whole patron object into the logs... having said that.. some of the log output I've since learnt depends on doing diffs at the point of display so need whole object dumps at the moment. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- See https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/Pa... for the JSON 'changed field' structure we opted for. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- and see https://git.koha-community.org/Koha-community/Koha/src/branch/master/koha-tm... for the jsdiff based diff we display. Removing whitespace will confuse that diff display I reckon... certainly worth checking for regressions there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I think we should grab the more general handling from the middle two patches on bug 28700 for here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125692|0 |1 is obsolete| | --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 127289 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127289&action=edit Bug 28700: Get from storage before log actions To make sure we have logging the values from DB. It will reduce DB action_log table size when we log the full object and it contains DateTime object. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- This patch is more or less the same as Petro, but 1. it calls get_from_storage and 2. deals with any Koha::Object we will pass. There is much more to do for logging correctly, this patch is only impacting calls where Dumper($object->unblessed) were passed to logaction. We certainly need to improve the whole thing and pass an object from everywhere, deal with logic in logaction (depending on the objects we have), log a pre/post stringified versions of the object in 2 different columns (to allow a nice diff from the UI), and finally stringify using JSON (that is currently done only for acquisition logs and patron modification, which is quite inconsistent). This is definitely for a separate bug report, but I think we should focus on it (as a team) for 22.05. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|RM_priority |rel_21_11_candidate Assignee|stalkernoid@gmail.com |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29413 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 127446 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127446&action=edit Bug 28692: Get from storage before log actions To make sure we have logging the values from DB. It will reduce DB action_log table size when we log the full object and it contains DateTime object. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127289|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=28692 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Great, this is a good first step and enough at this late stage in the cycle for the next release in my opinion. I'd have been tempted to drop the 'item' prepension, but that would lead to questions like "Should we run a DB update to fix existing cases" Signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Promising: Undefined subroutine &C4::Log::Dumper called at /usr/share/koha/C4/Log.pm line 81 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Martin Renvoize from comment #20)
Great, this is a good first step and enough at this late stage in the cycle for the next release in my opinion.
I'd have been tempted to drop the 'item' prepension, but that would lead to questions like "Should we run a DB update to fix existing cases"
Signing off.
How did you test it? First thing is a crash on missing module.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + $infos = Dumper( $infos->unblessed ); + + if ( $infos->isa('Koha::Item') && $modulename eq 'CATALOGUING' && $actionname eq 'MODIFY' ) { Ha. If you change the variable to a string, it wont be a Koha::Item anymore ;) Will fix it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Koha::Item Not sure if there was an idea to log the item version before we store (get_from_storage call) but we only save the current stage: my $result = $self->SUPER::store; if ( $log_action && C4::Context->preference("CataloguingLog") ) { $action eq 'create' ? logaction( "CATALOGUING", "ADD", $self->itemnumber, "item" ) : logaction( "CATALOGUING", "MODIFY", $self->itemnumber, $self ); } We already super stored. So if we wanted the version without the changes like the BIBLIO BEFORE then we should move the super store here? In that case I would also add a BEFORE. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- - my $result = $self->SUPER::store; if ( $log_action && C4::Context->preference("CataloguingLog") ) { $action eq 'create' ? logaction( "CATALOGUING", "ADD", $self->itemnumber, "item" ) : logaction( "CATALOGUING", "MODIFY", $self->itemnumber, $self ); } + my $result = $self->SUPER::store; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127446|0 |1 is obsolete| | --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127574 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127574&action=edit Bug 28692: Get from storage before log actions To make sure we have logging the values from DB. It will reduce DB action_log table size when we log the full object and it contains DateTime object. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #28 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127575 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127575&action=edit Bug 28962: (QA follow-up) Add module Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127576 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127576&action=edit Bug 28962: (QA follow-up) Sort hash keys of unblessed Trivial add. Reads much better. And might help future diffs. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #30 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127577 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127577&action=edit Bug 28962: (QA follow-up) Fix test for objects Simpler to check the ref instead of isa. Fix the bug where object is first stringified and then tested for being a Koha::Item. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #31 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Development and testing seems to be done too hastily. Three follow-ups ;) Comment25 and 26 leaving open. Note that I didnt have time to add some additional test for logaction. But you are welcome to still add it, Jonathan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #32 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #30)
Created attachment 127577 [details] [review] Bug 28962: (QA follow-up) Fix test for objects
Simpler to check the ref instead of isa. Fix the bug where object is first stringified and then tested for being a Koha::Item.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
This is not correct, we have modules in Koha:: that are not Koha::Object-based objects. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #33 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 127630 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127630&action=edit Bug 28692: Correctly prefix "item " for Koha::Item MODIFY logs or the isa won't work -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #34 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Comments 25 and 26 need more attention, and tests are missing. Help welcomed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #35 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #34)
Comments 25 and 26 need more attention, and tests are missing.
Help welcomed.
Revisiting it now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127575|0 |1 is obsolete| | Attachment #127576|0 |1 is obsolete| | Attachment #127577|0 |1 is obsolete| | Attachment #127630|0 |1 is obsolete| | --- Comment #36 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127639 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127639&action=edit Bug 28692: (QA follow-up) Add module Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #37 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127640 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127640&action=edit Bug 28692: (QA follow-up) Sort hash keys of unblessed Trivial add. Reads much better. And might help future diffs. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #38 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127641 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127641&action=edit Bug 28692: (QA follow-up) Fix test for objects Fix the bug where object is first stringified and then tested for being a Koha::Item. Note: simple test since even 2->isa('Some::Class') works fine. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127641|0 |1 is obsolete| | --- Comment #39 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127642 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127642&action=edit Bug 28692: (QA follow-up) Fix test for objects Fix the bug where object is first stringified and then tested for being a Koha::Item. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127574|0 |1 is obsolete| | --- Comment #40 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127645 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127645&action=edit Bug 28692: Get from storage before log actions To make sure we have logging the values from DB. It will reduce DB action_log table size when we log the full object and it contains DateTime object. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127639|0 |1 is obsolete| | --- Comment #41 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127646 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127646&action=edit Bug 28692: (QA follow-up) Add module Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127640|0 |1 is obsolete| | --- Comment #42 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127647 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127647&action=edit Bug 28692: (QA follow-up) Sort hash keys of unblessed Trivial add. Reads much better. And might help future diffs. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127642|0 |1 is obsolete| | --- Comment #43 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127648 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127648&action=edit Bug 28692: (QA follow-up) Fix test for objects Fix the bug where object is first stringified and then tested for being a Koha::Item. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #44 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 127649 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127649&action=edit Bug 28692: (follow-up) Add tests Test plan: Run t/db_dependent/Log.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #45 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #34)
Comments 25 and 26 need more attention, and tests are missing.
Help welcomed.
Added tests. It seems to me that we should postpone comments 25 and 26. They feel a bit out of scope here too. We want to reduce log size here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #46 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #32)
(In reply to Marcel de Rooy from comment #30)
Created attachment 127577 [details] [review] [review] Bug 28962: (QA follow-up) Fix test for objects
Simpler to check the ref instead of isa. Fix the bug where object is first stringified and then tested for being a Koha::Item.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
This is not correct, we have modules in Koha:: that are not Koha::Object-based objects.
Fixed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #47 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_21_11_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED CC| |kyle@bywatersolutions.com --- Comment #48 from Kyle M Hall <kyle@bywatersolutions.com> --- Doesn't apply to 21.05.x. Please rebase and let me know if you think this should be backported. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #49 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Kyle M Hall from comment #48)
Doesn't apply to 21.05.x. Please rebase and let me know if you think this should be backported.
It really should have :) My 20.11 logs were growing steadily with these datetime object dumps. The rebase was trivial.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28692 --- Comment #50 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- *** Bug 28700 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org