When looking at the messages SIP2 puts in syslog there are several messages where hashes are printed directly to the log, producing messages like these: Feb 11 09:00:23 koha koha_sip_koha[13575]: ILS::Checkout: patron 123 has checked out HASH(0x55a5b187f858), HASH(0x55a5b1896ad0), HASH(0x55a5b18a6cf0) Feb 11 09:00:25 koha koha_sip_koha[13575]: summary_info: list = (HASH(0x55a5b18aea60), HASH(0x55a5b18ae718), HASH(0x55a5b184ef48)... Not very useful?
They are debug messages, if syslog is not configured to log "debug" I would not expect them to be in the logs.
(In reply to Jonathan Druart from comment #1) > They are debug messages, if syslog is not configured to log "debug" I would > not expect them to be in the logs. Are they useful in any way? To me it just says "there were x number of hashes here", not sure if that makes me much wiser?
Indeed Magnus, I was missing your point (read too fast).
Created attachment 98740 [details] [review] Bug 24629: SIP log - Display barcode instead of hash
(In reply to Magnus Enger from comment #0) > Feb 11 09:00:23 koha koha_sip_koha[13575]: ILS::Checkout: patron 123 has > checked out HASH(0x55a5b187f858), HASH(0x55a5b1896ad0), HASH(0x55a5b18a6cf0) This one is weird, it seems that sometimes we have hash and itemnumber: Few lines before the syslog statement: 167 push( @{ $patron->{items} }, $item_id ); But, in renew: 458 foreach my $i (@{$patron->{items}}) { > Feb 11 09:00:25 koha koha_sip_koha[13575]: summary_info: list = > (HASH(0x55a5b18aea60), HASH(0x55a5b18ae718), HASH(0x55a5b184ef48)... Fixed with the patch.
(In reply to Jonathan Druart from comment #5) > (In reply to Magnus Enger from comment #0) > > > Feb 11 09:00:23 koha koha_sip_koha[13575]: ILS::Checkout: patron 123 has > > checked out HASH(0x55a5b187f858), HASH(0x55a5b1896ad0), HASH(0x55a5b18a6cf0) > > This one is weird, it seems that sometimes we have hash and itemnumber: > > Few lines before the syslog statement: > 167 push( @{ $patron->{items} }, $item_id ); > > But, in renew: > 458 foreach my $i (@{$patron->{items}}) { then we are using $i->{barcode}, so it is supposed to be a hashref.
The patch is an improvement, but there are still some HASH'es left. This one is interesting: Feb 14 12:40:05 kohaserver koha_sip[19050]: ILS::Checkout: patron x has checked out 52450820597 Feb 14 12:40:05 kohaserver koha_sip[19050]: ILS::Checkout: patron x has checked out HASH(0x5635b7193418), 80062844366 Feb 14 12:40:07 kohaserver koha_sip[19050]: ILS::Checkout: patron x has checked out HASH(0x5635b6f1ab80), HASH(0x5635b6f1a2c8), 80062908790 Feb 14 12:40:07 kohaserver koha_sip[19050]: ILS::Checkout: patron x has checked out HASH(0x5635b718c6c8), HASH(0x5635b6f31900), HASH(0x5635b6f20f20), 80059004255 x is the same in all 4 lines. I think this comes from ILS.pm, line 171.
Created attachment 99370 [details] [review] Bug 24629: SIP patron items contains an array of hashes $patron->{items} contains the borrowers checkouts as an array of hashes { barcode => $item->barcode } When printing to log we assumed these were only barcodes This patch pushes the current checkout as a hash and maps the values retrieved to a string To test: 1 - Enable SIP debug mode 2 - Perform multiple checkouts for a patron 3 - Note the messages like: koha koha_sip_koha[13575]: ILS::Checkout: patron 123 has checked out HASH(0x55a5b187f858), HASH(0x55a5b1896ad0), HASH(0x55a5b18a6cf0), 7826832 4 - Apply patch 5 - Restart all the things 6 - Do some checkouts via SIP 6 - Messages should now have barcodes
Setting status to NSO.
Created attachment 99478 [details] [review] Bug 24629: SIP log - Display barcode instead of hash Signed-off-by: Magnus Enger <magnus@libriotech.no>
Created attachment 99480 [details] [review] Bug 24629: SIP patron items contains an array of hashes $patron->{items} contains the borrowers checkouts as an array of hashes { barcode => $item->barcode } When printing to log we assumed these were only barcodes This patch pushes the current checkout as a hash and maps the values retrieved to a string To test: 1 - Enable SIP debug mode 2 - Perform multiple checkouts for a patron 3 - Note the messages like: koha koha_sip_koha[13575]: ILS::Checkout: patron 123 has checked out HASH(0x55a5b187f858), HASH(0x55a5b1896ad0), HASH(0x55a5b18a6cf0), 7826832 4 - Apply patch 5 - Restart all the things 6 - Do some checkouts via SIP 6 - Messages should now have barcodes Signed-off-by: Magnus Enger <magnus@libriotech.no> This patch and the one from Jonathan taken together seems to solve the problem, I no longer see messages with HASH(...).
Created attachment 99487 [details] [review] Bug 24629: SIP log - Display barcode instead of hash Signed-off-by: Magnus Enger <magnus@libriotech.no> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 99488 [details] [review] Bug 24629: SIP patron items contains an array of hashes $patron->{items} contains the borrowers checkouts as an array of hashes { barcode => $item->barcode } When printing to log we assumed these were only barcodes This patch pushes the current checkout as a hash and maps the values retrieved to a string To test: 1 - Enable SIP debug mode 2 - Perform multiple checkouts for a patron 3 - Note the messages like: koha koha_sip_koha[13575]: ILS::Checkout: patron 123 has checked out HASH(0x55a5b187f858), HASH(0x55a5b1896ad0), HASH(0x55a5b18a6cf0), 7826832 4 - Apply patch 5 - Restart all the things 6 - Do some checkouts via SIP 6 - Messages should now have barcodes Signed-off-by: Magnus Enger <magnus@libriotech.no> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Trivial fix, thanks guys.. PQA
(In reply to Martin Renvoize from comment #14) > Trivial fix, thanks guys.. > > PQA That is not that trivial in my opinion. We should track down a bit where does come from (and the possible problems) the 2 different types in the item list (scalar vs hashref). I think I asked Nick last Friday but cannot remember the conclusion.
Nice work everyone! Pushed to master for 20.05
(In reply to Jonathan Druart from comment #15) > (In reply to Martin Renvoize from comment #14) > > Trivial fix, thanks guys.. > > > > PQA > > That is not that trivial in my opinion. > We should track down a bit where does come from (and the possible problems) > the 2 different types in the item list (scalar vs hashref). > > I think I asked Nick last Friday but cannot remember the conclusion. Did you manage to clear this one up?
(In reply to Katrin Fischer from comment #17) > (In reply to Jonathan Druart from comment #15) > > (In reply to Martin Renvoize from comment #14) > > > Trivial fix, thanks guys.. > > > > > > PQA > > > > That is not that trivial in my opinion. > > We should track down a bit where does come from (and the possible problems) > > the 2 different types in the item list (scalar vs hashref). > > > > I think I asked Nick last Friday but cannot remember the conclusion. > > Did you manage to clear this one up? I added a second patch, it was another place where we had a more complex object than assumed