| Time |
Nick |
Message |
| 00:00 |
|
sandbergja joined #evergreen |
| 06:01 |
pinesol |
News from qatests: Testing Success <http://testing.evergreen-ils.org/~live> |
| 07:18 |
|
rjackson_isl_hom joined #evergreen |
| 08:02 |
|
collum joined #evergreen |
| 08:29 |
|
mantis joined #evergreen |
| 08:34 |
|
rfrasur joined #evergreen |
| 08:38 |
|
mmorgan joined #evergreen |
| 09:03 |
|
Dyrcona joined #evergreen |
| 09:07 |
|
jvwoolf1 joined #evergreen |
| 11:25 |
|
laurie joined #evergreen |
| 11:51 |
|
jihpringle joined #evergreen |
| 11:56 |
|
dbwells joined #evergreen |
| 12:16 |
|
jamesrf joined #evergreen |
| 12:19 |
Dyrcona |
jeffdavis: Going to back to your thing with holds counts yesterday, it looks like the real culprit is open-ils.actor.user.hold_requests.count. I've only tested it on Perl 5.30 so far, and it returns the holds available count as a string. |
| 12:24 |
Dyrcona |
Seems bizarre that it would do that, though. The total field is returned as a number, and the way they're calculated *should* be functionally equivalent. |
| 12:24 |
Dyrcona |
I have a suspicion of how to fix it, though. |
| 12:25 |
jeffdavis |
I'm happy to test any ideas you might have. :) |
| 12:32 |
Dyrcona |
Hm... eg_db_config through some errors for me on Ubuntu 18.04. I need to check that before I can go on. |
| 12:35 |
Dyrcona |
OK. I have to install some prerequisites. |
| 12:43 |
Dyrcona |
In Perl 5.26 it's a number. That's really bizarre and must be some kind of Perl bug. |
| 12:43 |
|
dbwells joined #evergreen |
| 12:44 |
|
collum joined #evergreen |
| 12:50 |
Dyrcona |
And, it doesn't happen with a little Perl script intended to test the problem. so it's more complicated than it appears at first glance. |
| 12:54 |
Dyrcona |
So, my "fix" resolves it, but I think it's just patching over the real problem, which I suspect is buried in OpenSRF somewhere. I was not able to reproduce the behavior with a simple Perl script. |
| 12:55 |
Dyrcona |
We should have to go adding int() around all of our uses of scalar(@array)... |
| 12:56 |
Dyrcona |
s/should/shouldn't/ |
| 12:57 |
|
dbwells joined #evergreen |
| 12:59 |
Dyrcona |
Without patching, the behind_desk value also comes out as a string if you add the context org unit argument. |
| 13:01 |
Dyrcona |
So, I suspect a bug in OpenSRF or in the JSON code, rather than in Perl 5.30, though it has to be related to 5.30. |
| 13:01 |
Dyrcona |
It's probably another case of us relying on some old Perl behavior that changed, without us realizing it. |
| 13:02 |
Dyrcona |
On Ubuntu 18.04, this all OK. |
| 13:03 |
Dyrcona |
Not sure how to write this up for Launchpad..... |
| 13:08 |
jeff |
which perl version works? |
| 13:08 |
jeff |
ah, jeffdavis said "Perl v5.30.0 vs v5.26.1" |
| 13:10 |
Dyrcona |
5.26.1 on Ubuntu 18.04 works. 5.30 produces the strings, but if I patch with int() it works with 5.30, but I think the bug is actually more subtle and hidden somewhere else because I can reproduce it with simple Perl code. |
| 13:10 |
jeff |
JSON::PP or JSON::XS changes would be worth looking into. |
| 13:10 |
Dyrcona |
s/can/can't/ |
| 13:10 |
Dyrcona |
Yes, definitely. |
| 13:12 |
jeff |
JSON::PP has been upgraded from version 2.97001 to 4.02. |
| 13:12 |
jeff |
JSON::PP as JSON::XS 4.0 enables allow_nonref by default. |
| 13:12 |
jeff |
https://perldoc.perl.org/5.30.0/perldelta |
| 13:13 |
Dyrcona |
Might be it. I'll see if I can find where we use JSON to turn off allow_nonref. |
| 13:14 |
jeffdavis |
Seems like it might only be an issue where ready = 0? For a user with 1 or more holds ready, the result is an int rather than a string. |
| 13:15 |
Dyrcona |
I don't think that's it after reading the documentation, unless there's a bug with that setting. |
| 13:15 |
Dyrcona |
jeffdavis: I only tested with a handful of concerto patrons, none of whom turned out to have holds. |
| 13:16 |
jeff |
also interesting, https://metacpan.org/source/ISHIGAKI/JSON-4.03/Changes#L25 "allow PERL_JSON_PP_USE_B environmental variable to restore old number detection behavior for compatibility" |
| 13:17 |
Dyrcona |
We already turn allow_nonref on. |
| 13:17 |
jeff |
and: |
| 13:17 |
jeff |
tweak internal number detector to always considers a flagged value as a string (GH#35, haarg++) |
| 13:19 |
jeff |
https://github.com/makamaka/JSON/issues/35 is the referenced issue |
| 13:21 |
jeff |
might be the referenced issue. github issue numbers are... small. |
| 13:25 |
jeffdavis |
Seems likely. |
| 13:26 |
jeffdavis |
Or rather the comments about JSON::XS specifically sound like our issue. |
| 13:26 |
Dyrcona |
This is where a Perl REPL would come in handy. |
| 13:27 |
Dyrcona |
jeffdavis: I don't think so. |
| 13:28 |
pastebot |
"Dyrcona" at 168.25.130.30 pasted "Using JSON::XS simply." (5 lines) at http://paste.evergreen-ils.org/10128 |
| 13:28 |
jeff |
https://bugs.launchpad.net/opensrf/+bug/1257264 is an example of where we dealt with a similar issue. |
| 13:28 |
pinesol |
Launchpad bug 1257264 in OpenSRF 2.1 "JSON::XS 3.0 changes the way booleans are represented" [Medium,Fix committed] |
| 13:29 |
jeff |
"Simple Perl scalars (any scalar that is not a reference) are the most difficult objects to encode: JSON::XS will encode undefined scalars as JSON null values, scalars that have last been used in a string context before encoding as JSON strings, and anything else as number value:" |
| 13:29 |
jeff |
https://metacpan.org/pod/JSON::XS#PERL-%3E-JSON |
| 13:31 |
pastebot |
"Dyrcona" at 168.25.130.30 pasted "Also on Ubuntu 20" (4 lines) at http://paste.evergreen-ils.org/10129 |
| 13:33 |
Dyrcona |
Uh-huh. But look at what I pasted, and it's doing the right thing. |
| 13:33 |
Dyrcona |
Both the pastes were don one Ubuntu 20.04. |
| 13:37 |
pastebot |
"Dyrcona" at 168.25.130.30 pasted "Not even this" (5 lines) at http://paste.evergreen-ils.org/10130 |
| 13:37 |
|
mantis left #evergreen |
| 13:38 |
Dyrcona |
That last paste is pretty much what the code in open-ils.actor.user.hold_requests.count does, so the bug is more subtle than it first appears. |
| 13:38 |
jeff |
did someone say that it only happens when an org unit is passed? |
| 13:38 |
JBoyer |
Does any of the code that works on the ready_holds value at any point use == ? Looking at jeff's link all you have to do is make a string comparison (by using the "wrong" comparison operator; I'll spare everyone my tight 5 min rant on types) to trigger this. |
| 13:39 |
jeff |
which value ends up being quoted? |
| 13:39 |
* JBoyer |
isn't really able to dig (much) deep right now but that certainly stood out |
| 13:39 |
jeff |
sorry, I'm jumping in in the middle of this. |
| 13:39 |
* jeff |
scrolls back |
| 13:39 |
Dyrcona |
The function in question begins at line 2153 of Actor.pm. |
| 13:39 |
JBoyer |
jeff, ready_holds, which then causes the alert screen to pop up for every patron |
| 13:39 |
|
mantis joined #evergreen |
| 13:39 |
Dyrcona |
jeff: ready and behind_desk when present. |
| 13:39 |
jeff |
12:59:56 <+Dyrcona> Without patching, the behind_desk value also comes out as a string if you add the context org unit argument. |
| 13:40 |
jeff |
so, that's likely because: |
| 13:40 |
jeff |
$U->is_true($_->{behind_desk}) |
| 13:40 |
Dyrcona |
That's in the grep condition. |
| 13:41 |
Dyrcona |
The value being set to scalar is an empty array. |
| 13:41 |
jeff |
is_true does this: |
| 13:41 |
jeff |
return 1 if $item and $item !~ /^f$/i; |
| 13:41 |
jeff |
but I don't know if that's enough to tag the passed value. |
| 13:44 |
Dyrcona |
The grep for ready does NOT use is_true, and it has the same problem. |
| 13:44 |
Dyrcona |
It's probably something about using grep. |
| 13:44 |
Dyrcona |
I used to like Perl, but not for quite a while, now. |
| 13:46 |
Dyrcona |
If I wrap those scalar(grep ...) with int(), the results are correct, but I think that's just papering over a deeper issue. |
| 13:55 |
Dyrcona |
Hmm. I'll bet it's actually scalar(undef) and not scalar(())... |
| 13:57 |
Dyrcona |
Eh, nope. That's still giving me an unquoted 0 on Ubuntu 20.04 when going through OpenSRF::Utils::JSON. |
| 14:02 |
pastebot |
"Dyrcona" at 168.25.130.30 pasted "It's not just the grep, either." (5 lines) at http://paste.evergreen-ils.org/10131 |
| 14:15 |
|
collum joined #evergreen |
| 14:50 |
Dyrcona |
Yeah, I've tried increasingly more sophisticated programs, including building an oilsMessage and jamming it into an array that gets converted to JSON in the same way that OpenSRF::AppSession does, and I still don't get strings.... |
| 14:51 |
Dyrcona |
I even tried double-encoding it with perl2JSON. |
| 14:52 |
|
mantis left #evergreen |
| 14:54 |
|
mantis joined #evergreen |
| 14:54 |
jeff |
I see a difference with Perl 5.32.1 with JSON::PP 4.04 vs JSON::XS 4.03. I see the documented behavior of scalars with a string context (where there's a PV and FLAGS includes PV/PVOK): the value is quoted. |
| 14:58 |
jeff |
Dyrcona: what version of JSON::XS are you running on your Ubuntu 18.04 system? |
| 15:02 |
Dyrcona |
jeff: This is Ubuntu 20.04 where I'm testing, the one that show the bug. |
| 15:02 |
jeff |
yep, got that. |
| 15:03 |
* jeff |
looks on packages.ubuntu.com |
| 15:03 |
Dyrcona |
Ubuntu 20.04 has 4.02 |
| 15:03 |
Dyrcona |
Ubuntu 18.04 has 3.02. |
| 15:04 |
Dyrcona |
I see the bug when I use srfsh to make the request, but trying to emulate what looks like the problematic code on 20.04 turns up nothing. |
| 15:04 |
Dyrcona |
I updated all packages before testing today. |
| 15:06 |
Dyrcona |
I have 5.32.1 on a FreeBSD server. |
| 15:07 |
pastebot |
"Dyrcona" at 168.25.130.30 pasted "Here's my latest test" (10 lines) at http://paste.evergreen-ils.org/10132 |
| 15:11 |
Dyrcona |
I get identical results on both Ubuntu servers with the above. I don't have Evergreen on the FreeBSD machine, so I won't test it there. |
| 15:12 |
Dyrcona |
Also, don't have JSON::XS on FreeBSD... |
| 15:12 |
Dyrcona |
I could install it, but this isn't a machine that I use for development. |
| 15:13 |
Dyrcona |
Maybe I should switch from ZZ Top to Jefferson Airplane.... :) |
| 15:15 |
mantis |
running 3.5.4 on a test server and images are failing to load in the catalog |
| 15:23 |
Dyrcona |
mantis: All images or just added content/book jackets? |
| 15:25 |
mantis |
content jackets |
| 15:26 |
mantis |
I'm not sure why that happened |
| 15:26 |
Dyrcona |
Did you configure the added content provider in opensrf.xml? |
| 15:26 |
Dyrcona |
I think that's where it goes.... |
| 15:36 |
jeff |
okay, I think I have a simple (yet still relevant) test that demonstrates a difference in behavior between Perl 5.26.3 and Perl 5.32.1, both with JSON::XS 4.03. I'm going to try and narrow down where it changes. |
| 15:38 |
Dyrcona |
jeff: Can you paste the test? I'd like to see it. |
| 15:38 |
mantis |
Dyrcona: thanks I'll take a look at it |
| 15:38 |
mantis |
Dyrcona++ |
| 15:39 |
|
mantis left #evergreen |
| 15:41 |
jeff |
here's the test and output under 5.26.3 and 5.32.1 so far, with some Devel::Peek Dump() output to help see where the flags are: https://gist.github.com/jeff/6ab1d188db2cd21ac1d45cf06544d7d2 |
| 15:42 |
jeff |
I'm installing 5.28 and 5.30 to help identify where the change was introduced. |
| 15:45 |
Dyrcona |
jeff: Interesting. My tests haven't done that. |
| 15:45 |
|
dbwells joined #evergreen |
| 15:48 |
miker |
oh, my, that's terrible if scalar() returns strings now |
| 15:49 |
jeff |
miker: not obvious in that simplified test, but it seems to only be when we have an empty array. |
| 15:50 |
Dyrcona |
Yeah, but I can't make it do that. jeff's test does for me, but my tests don't. |
| 15:53 |
jeff |
5.28.3 does it also. |
| 15:53 |
jeff |
updated gist with new output |
| 15:53 |
|
jihpringle joined #evergreen |
| 15:56 |
jeff |
demonstration of nonempty array not running into the new/problematic behavior: https://gist.github.com/jeff/7cb4823506ca0f35e620a3e18868f52c |
| 15:57 |
|
collum joined #evergreen |
| 16:00 |
miker |
so somewhere between 5.26 and 5.28, scalar(@empty) changed and quotes the count... |
| 16:01 |
jeff |
seems so. |
| 16:02 |
jeff |
looking at https://perldoc.perl.org/5.28.0/perldelta I see no clear culprits, though this caught my eye: |
| 16:02 |
miker |
so do we just need to spell it 0+scalar(@bar) then? |
| 16:02 |
jeff |
> A new kind of magic scalar, called a "nonelem" scalar, has been introduced. It is stored in an array to denote a non-existent element, whenever such an element is accessed in a potential lvalue context. |
| 16:02 |
jeff |
but I'm not sold on that being related. |
| 16:04 |
miker |
changing jeff's test to that (cast to number by prepending 0+ ) "fixes" foo for me on 5.28 |
| 16:04 |
Dyrcona |
miker: int(scalar(...)) works, too. |
| 16:05 |
miker |
Dyrcona: yeah, either way ... s/scalar\(/0+scalar(/g in vim is easier though ;) |
| 16:06 |
Dyrcona |
What's weird is I'm not seeing it with the scripts that I pasted earlier. |
| 16:08 |
|
jihpringle joined #evergreen |
| 16:10 |
miker |
I don't see any uses of scalar() in opensrf-proper that could cause internal errors due to this (and wouldn't expect to) nor anything that's sent out of the process that could be from an empty array ... so at least there's that :) |
| 16:11 |
Dyrcona |
miker: It turns up in open-ils.actor.user.hold_requests.count |
| 16:11 |
Dyrcona |
Also, looks like anyone using Debian Buster in production would see this, too. |
| 16:12 |
miker |
Dyrcona: I understand, I meant internal to opensrf perl code, not evergreen application code (looking for Worse Problems :) ) |
| 16:13 |
Dyrcona |
Right. |
| 16:16 |
jeff |
Dyrcona: that is interesting that your tests using OpenSRF libs aren't showing the problematic behavior. I modified one of your recent ones to use JSON::XS directly and it shows the same issue with the same versions (no surprise, since it's almost the same as one of my tests now): https://gist.github.com/jeff/a4fd184305f0a7870258040e3e3ce702 |
| 16:18 |
jeff |
"Various integer-returning ops are now more efficient in scalar/boolean context." |
| 16:18 |
jeff |
(also from perl5280delta) |
| 16:18 |
jeff |
"Converting a single-digit string to a number is now substantially faster." |
| 16:20 |
Dyrcona |
jeff: I didn't get it when I used the JSON::XS object interface, either. |
| 16:23 |
Dyrcona |
So, should we add this to Lp? |
| 16:24 |
miker |
jeff: "... faster, because it doesn't happen" ;) |
| 16:36 |
pastebot |
"jeffdavis" at 168.25.130.30 pasted "@b vs my @b" (4 lines) at http://paste.evergreen-ils.org/10133 |
| 16:43 |
Dyrcona |
Yeah, "my" seems to make a difference. I missed it earlier because the dumper used single quotes. |
| 17:09 |
jeff |
yeah, that's... fun. |
| 17:10 |
jeff |
Dump of scalar(@array) vs scalar(my @array) across the four versions of Perl I have handy: https://gist.github.com/jeff/5419d2980da092ed5cfb97cbaf43b64e |
| 17:16 |
jeff |
added a version with my $v = scalar(my @array); Dump($v);, which brings the REFCNT down from 2147483647 to 1. |
| 17:20 |
miker |
that's an oddly sus refcnt. almost like an underflow bug. wheee! |
| 17:23 |
jeff |
i think the REFCNT being INT_MAX may simply reflect the fact that the "variable" being dumped is not a variable but is the output of calling something in a mostly-void context. |
| 17:24 |
jeff |
with Dump(scalar(my @array)), the value isn't being stored anywhere... |
| 17:24 |
jeff |
but it's interesting that that also changes between 5.26 and 5.28. |
| 17:30 |
jeff |
scalar(my %hash) doesn't seem to do it. just scalar(my @array). interesting. |
| 17:30 |
jeff |
(and annoying) |
| 17:37 |
|
mmorgan left #evergreen |
| 17:46 |
jeffdavis |
Python 3 also makes installing translations "fun" on Ubuntu 20.04. |
| 17:46 |
jeffdavis |
Starting to wonder if we should stick with 18.04 for our upgrade. :( |
| 18:01 |
pinesol |
News from qatests: Testing Success <http://testing.evergreen-ils.org/~live> |
| 18:42 |
|
sandbergja joined #evergreen |
| 19:45 |
|
dbwells joined #evergreen |
| 22:09 |
|
sandbergja joined #evergreen |
| 22:15 |
|
dbwells joined #evergreen |
| 22:47 |
* jeff |
bisects |
| 22:58 |
|
dbwells joined #evergreen |
| 23:14 |
|
dbwells joined #evergreen |
| 23:16 |
jeff |
Perl commit 7be75ccf16313d987eb5a6e9ff6aec9fea4ef3d4 is where the behavior changed. |
| 23:22 |
jeff |
https://github.com/Perl/perl5/commit/7be75ccf16313d987eb5a6e9ff6aec9fea4ef3d4 |
