| Time |
Nick |
Message |
| 07:29 |
|
collum joined #evergreen |
| 08:00 |
|
BDorsey joined #evergreen |
| 08:02 |
|
kworstell-isl joined #evergreen |
| 08:11 |
|
rfrasur joined #evergreen |
| 08:41 |
|
mmorgan joined #evergreen |
| 08:45 |
|
sandbergja joined #evergreen |
| 09:12 |
|
Dyrcona joined #evergreen |
| 09:21 |
|
collum joined #evergreen |
| 09:32 |
Dyrcona |
Today's test is going better than yesterday's. It is still chugging away at the events, and I apparently have not run out of drones. |
| 09:34 |
Dyrcona |
I'm using our production server's configuration for this test. |
| 09:35 |
Dyrcona |
Modulo the database connection parameters, etc., of course. |
| 09:36 |
Stompro |
Morning all, I'm looking into cleaning up our /openils/var/web/reporter data this morning. Every report generated since we migrated in 2015 exists in there. Is there an existing evergreen method for purging those? |
| 09:47 |
Stompro |
Bug 1835317 talks about adding a retention interval to report output. |
| 09:47 |
pinesol |
Launchpad bug 1835317 in Evergreen "Report output should have a retention interval" [Wishlist,Confirmed] https://launchpad.net/bugs/1835317 |
| 09:48 |
Dyrcona |
Stompro: I use find from cron: find /mnt/evergreen/reports/ -type f -mtime +90 -delete |
| 09:49 |
Stompro |
Dyrcona, do you also clean up reporter.completed_reports ? |
| 09:49 |
Dyrcona |
And another one to delete empty directories: find /mnt/evergreen/reports/ -empty -type d -delete |
| 09:54 |
jeff |
fancy newfangled -delete option... I still find my fingers using -print0 | xargs -0 :-) |
| 09:54 |
Dyrcona |
Stompro: Yes, we cleanup old reqports in the database: https://pastebin.com/QQanSgQs |
| 09:54 |
Dyrcona |
New-fangled? :) |
| 09:59 |
Stompro |
Dyrcona++ thanks |
| 10:00 |
pinesol |
News from commits: LP#2015758 - marc_export set default encoding to UTF-8 <https://git.evergreen-ils.org/?p=Evergreen.git;a=commitdiff;h=7234c637778da729061ee61f9327a95b4d80864e> |
| 10:01 |
Dyrcona |
I suppose -delete is an extension. It's not in POSIX. |
| 10:03 |
jeff |
it isn't. also, it's new! added in 2004! that's barely... yeah, well... a few years ago now, I guess. |
| 10:04 |
jeff |
(at least ten.) |
| 10:05 |
jeff |
:-) |
| 10:05 |
Dyrcona |
Heh. |
| 10:05 |
Dyrcona |
Works on FreeBSD and OpenBSD, too, so that's good enough for me. :) |
| 10:07 |
|
smayo joined #evergreen |
| 10:09 |
JBoyer |
jeff, just wait until you hear about -daystart ;p |
| 10:13 |
Dyrcona |
:) |
| 10:14 |
jeff |
JBoyer: surely you mean -fulldays |
| 10:21 |
sharpsie |
@dunno add and don't call me Shirley! |
| 10:21 |
pinesol |
sharpsie: The operation succeeded. Dunno #81 added. |
| 10:22 |
Dyrcona |
@dunno |
| 10:22 |
pinesol |
Dyrcona: Down time is a fact of business when you're a poor 501c3 corporation. |
| 10:22 |
JBoyer |
Google does not wish me to know when / if -fulldays has been or is a valid find option. |
| 10:23 |
sharpsie |
pinesol: what don't you know? |
| 10:23 |
pinesol |
sharpsie: connect to host dev port 22: Connection refused |
| 10:26 |
Dyrcona |
Heh. The manual page for GNU find has a "NON-BUGS" section. |
| 10:27 |
jeff |
JBoyer: it was renamed in 1991. I don't think there was any provision to keep the old option for compatibility. |
| 10:28 |
JBoyer |
jeff++ |
| 10:29 |
jeff |
i went looking for context to see how old -daystart was. looks like it was present as -fulldays since at least 1987, but that's going only from the changelog. history available in git for GNU findutils only seems to go back to 1996 (guessing it was converted from cvs, and before then I'm not sure what) |
| 10:31 |
|
dguarrac joined #evergreen |
| 10:32 |
Dyrcona |
RCS locally probably... With patches getting sent to mailing lists. |
| 10:35 |
Dyrcona |
Or maybe just versioned backup files: xxxx.~1~ xxx.~2~ and so on. :) |
| 10:54 |
|
briank joined #evergreen |
| 10:58 |
Bmagic |
is there any reason not to use local $@; before an eval statement within a perl module in Evergreen? |
| 10:59 |
Bmagic |
I see a lot of eval examples in the Evergreen perl moddules, but none of them are preceeded with local $@; |
| 11:28 |
jeffdavis |
I'm no Perl expert but I suspect we'd be better off just not using eval? |
| 11:30 |
berick |
sometimes we use eval as an alternate form of try/catch which is handy. |
| 11:30 |
berick |
the perl Error module has some drawbacks, iirc, at least it used to |
| 11:31 |
Bmagic |
eval statements are of plenty in the Evergreen codebase. I used the OpenSRF:Ex try statements, but in my case, the Error thrown isn't compatible with the Error object stack (at least that's my theory because the code breaks Evergreen when using try/catch) - but eval{} or do {} works fine |
| 11:31 |
berick |
we're not generally eval'ing strings |
| 11:32 |
Bmagic |
well, here's the details |
| 11:32 |
Bmagic |
try { |
| 11:32 |
Bmagic |
$U->simplereq('open-ils.circ', |
| 11:32 |
Bmagic |
'open-ils.circ.hold_reset_reason_entry.create',$self->editor->authtoken, $_->{id},OILS_HOLD_BETTER_HOLD); |
| 11:32 |
Bmagic |
} |
| 11:32 |
Bmagic |
catch Error with { |
| 11:32 |
Bmagic |
$logger->error("circulate: create reset reason failed with ".shift()); |
| 11:32 |
Bmagic |
}; |
| 11:32 |
Bmagic |
throws this bomb: circulate: create reset reason failed with " (perhaps you forgot to load "[ERR :345569:Circulate.pm:3327:16952157234738155] circulate: create reset reason failed with "?) at /usr/local/share/perl/5.30.0/OpenILS/Application/Circ/Circulate.pm line 3326. |
| 11:34 |
berick |
nice |
| 11:34 |
berick |
though in this case, why would you want to wrap a simplereq in a try/catch or eval? the API should return an error, not die outright |
| 11:35 |
Bmagic |
at first I thought it was the usage of shift() on the end of a string like that. But that turned out to be fine. Then I tried my $er = shift; $er = $er->stringify(); <- that worked but I felt better about using an eval instead, just in case the object thrown wasn't stringify() - able |
| 11:36 |
berick |
+1 to evel |
| 11:36 |
berick |
er, eval |
| 11:36 |
Bmagic |
berick: I was wondering that too, I think the developer was trying to be extra cautions about not letting the program die just because of this line |
| 11:36 |
berick |
*nod* |
| 11:37 |
Bmagic |
it's this feature https://bugs.launchpad.net/evergreen/+bug/2012669 |
| 11:37 |
pinesol |
Launchpad bug 2012669 in Evergreen "Hold request reset entries" [Wishlist,Confirmed] |
| 11:37 |
Bmagic |
here's the broken code: https://dropbox.mobiusconsortium.org/pickup/code_changes.html |
| 11:38 |
Bmagic |
here's the stringify version https://dropbox.mobiusconsortium.org/pickup/hold_code_changes_stringify.html |
| 11:38 |
Bmagic |
and here is the fix that I think is best https://dropbox.mobiusconsortium.org/pickup/hold_code_fix.html |
| 11:40 |
Dyrcona |
Here's a (useless at the moment) thought: Stop using Perl and use a language with actual exceptions or conditions and code built in to deal with it. |
| 11:40 |
Bmagic |
Dyrcona++ # I think that all the time |
| 11:41 |
Bmagic |
on the other hand, the more I work with Perl, the more I like it, and ignore it's idiosyncrasies |
| 11:42 |
Bmagic |
any objections to that last link there? Other than changing the error variable name to something more like "tmp_error" ? |
| 11:44 |
berick |
Bmagic: you're propsing the eval patch on the left side? |
| 11:44 |
Bmagic |
When I found out that the Ubuntu operating system relies on perl, I gained respect for it. I found out the hard way that the OS needed it, by deleting the whole perl folder in a bare bones container |
| 11:44 |
berick |
heh |
| 11:45 |
Bmagic |
berick: yep, left side. And, that brings me back to the original question: local $@; probably needs dropped |
| 11:46 |
berick |
IMO, if simplereq() fails, you have bigger problems than a single failed API call. that means your system is busted and your code should probably die. |
| 11:46 |
berick |
hence my objection to using eval at all in this scenario |
| 11:47 |
Bmagic |
standard examples of using eval {} or do {}; on the internet always includes that local $@ statement. Which I understand makes a local scoped version of the perl error variable. But we're not doing that anywhere else in our perl eval examples |
| 11:48 |
Bmagic |
berick: fair enough, this patch includes about 7 try/catches elsewhere https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/blake/lp_2012669_hold_reset_reason_main_branch |
| 11:48 |
berick |
+1 to 'local $@' if that's standard - i wasn't aware of it, my bad |
| 11:49 |
Dyrcona |
I used to really like Perl. Now, I don't. |
| 11:51 |
Bmagic |
berick: I'm thinking no on the local statement. Because it's not used anywhere else. Unless you think we need to update all* of our eval's throughout to have it. Which doesn't seem to make sense, because it's working as-is |
| 11:52 |
berick |
i can see why it's recommended, but agree we probably don't need to update the existing code |
| 11:52 |
Dyrcona |
FYI, local works that way for all Perl variables. |
| 11:53 |
Bmagic |
right, it's a scoping thing |
| 11:53 |
Bmagic |
I assumed it was to minimize the risk of altering the "other" $@ variable outside of this block, so other code can have their own error variable |
| 11:54 |
berick |
Bmagic: i'd argue most of those try/catch's are unecessary |
| 11:54 |
Bmagic |
without the local statement, I assume that any changes (errors) to $@ would be global and could* mess up the calling code if they are using $@ also |
| 11:55 |
Bmagic |
berick: but not all? |
| 11:55 |
berick |
i didn't read the whole patch :) |
| 11:55 |
berick |
sec |
| 11:57 |
Bmagic |
I'm about to squash the top two commits and force push |
| 11:58 |
Dyrcona |
Bmagic: You're talking about the stringify and eval commits being squashed? |
| 11:58 |
Bmagic |
Dyrcona: the top two here: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/blake/lp_2012669_hold_reset_reason_main_branch |
| 11:59 |
berick |
looks like they all wrap a simplereq or other API call -- none of those need try/catch's -- APIs return non-explosive errors on failure, unless the system as a whole is borked |
| 11:59 |
Bmagic |
better look quick, about to overwrite |
| 11:59 |
Dyrcona |
Also, I usually use if ($@) {} not the 'or do {}' syntax, but I think they come out the same. |
| 11:59 |
berick |
Dyrcona: same |
| 12:00 |
|
BAMkubasa joined #evergreen |
| 12:00 |
Dyrcona |
If those commits exist as-is in another branch, I don't have an issue with them being squashed in your collab branch. |
| 12:00 |
Dyrcona |
It's someone else's code, so be respectful. (Whatever that means.) :) |
| 12:02 |
Dyrcona |
Another fun thing is not all backend calls throw errors like that. Some return them. |
| 12:03 |
Bmagic |
This is a new API call from this patch, and it can throw an error sometimes. open-ils.circ.hold_reset_reason_entry.create is introduced in this patch |
| 12:03 |
Dyrcona |
We need to fix our API to have more standard conventions. |
| 12:03 |
Dyrcona |
So, it looks like we don't want a failure to create a reset reason to be fatal. |
| 12:04 |
Bmagic |
right, it's auxillary data, non-crucial to the function of Evergreen |
| 12:05 |
Bmagic |
if we fail to write a row to the tracking database, so be it, please continue with your regularly scheduled programming |
| 12:05 |
Dyrcona |
Right. Makes sense. |
| 12:06 |
Bmagic |
worse case, the staff won't know the reason that a hold was retargeted when looking at the history tracking |
| 12:07 |
Bmagic |
and we're not going to want Evergreen to completely crash for that reason |
| 12:07 |
Dyrcona |
I should read the bug description before I comment on the functionality. :) |
| 12:07 |
jeffdavis |
In your diff, aren't you localizing $@ for (the remainder of) the entire attempt_checkin_hold_capture function, not just for your eval? Is that what you want there? |
| 12:08 |
Bmagic |
right, it would scope it for all of the lines following the local statement. What would be the "fix" for that? Unscope it somehow? |
| 12:08 |
berick |
Bmagic: the API will return some kind of error info in that case, but it won't cause a crash on the client side |
| 12:08 |
berick |
it will just be a response that, say, needs to get logged |
| 12:08 |
Dyrcona |
Through braces around it: { .... } |
| 12:09 |
Dyrcona |
s/Through/Throw |
| 12:09 |
Bmagic |
I was thinking the same thing, wrapping the whole thing in curlies would keep the local even more local |
| 12:10 |
jeffdavis |
The Try::Tiny docs have some useful background about edge cases with this stuff: https://metacpan.org/pod/Try::Tiny#BACKGROUND |
| 12:10 |
|
jihpringle joined #evergreen |
| 12:10 |
Bmagic |
berick: pretty sure in a previous revision of this patch, it wasn't wraped in try/catch, and it would break evergreen occationally |
| 12:10 |
berick |
similarly, if the API is expected to fail somtimes, it should handle that scenario and return a meaningful error, not fall over |
| 12:11 |
berick |
Bmagic: then that code is the problem |
| 12:11 |
berick |
and needs to be smarter |
| 12:11 |
Bmagic |
right, so, move the try/catch into the api |
| 12:11 |
Bmagic |
I like that better |
| 12:11 |
berick |
no, make the api not die |
| 12:11 |
berick |
but also.. APIs don't return messages that cause clients to crash. only if the client does something funkh with the response it receives |
| 12:12 |
berick |
like deref a var that was not returned because the API failed and returned something else |
| 12:12 |
Dyrcona |
Also, where's the transaction created for that $e->commit on line 2285? I don't see it. |
| 12:12 |
Dyrcona |
Is the editor always_xact? |
| 12:12 |
Dyrcona |
Oh, never mind. It is. |
| 12:12 |
Dyrcona |
:P |
| 12:13 |
Dyrcona |
I'm having one of those days. I have to rerun a test of marc_export because I overwrote one of the output files with the data that I wanted to compare it to. |
| 12:15 |
berick |
Bmagic: but also... working code wins, so don't let me mood derail everything ;) feeling ranty this a.m. |
| 12:16 |
berick |
also, apparently, talking like a pirate |
| 12:17 |
Bmagic |
berick: I think I agree that the API shouldn't crash |
| 12:17 |
|
tlittle joined #evergreen |
| 12:17 |
Bmagic |
that's the case all over Evergreen, the caller doesn't need to baby the API call |
| 12:22 |
berick |
exactly |
| 12:22 |
berick |
opensrf runs APIs in a try/catch already |
| 12:23 |
berick |
so if they explode on the server, the server process won't die |
| 12:23 |
berick |
and it can package up the error info in a status message that goes back to the caller |
| 12:25 |
Bmagic |
berick: not sure how, but this API does crash unders certain circumstances. Rarely, but it can happen. Not sure how the error circumvents opensrf error handling |
| 12:26 |
Dyrcona |
Well, if the error condition doesn't work with Error and try/catch that might be the explanation. |
| 12:26 |
Bmagic |
might have been a race condition upon the DB |
| 12:27 |
Dyrcona |
That code updates a bunch of holds at once, doesn't it? It also gets called from the hold targeter, too, right? |
| 12:28 |
berick |
Bmagic: the API is crashing or the calling code is crashing? |
| 12:30 |
Bmagic |
API I believe |
| 12:30 |
Dyrcona |
Does a drone die? |
| 12:31 |
Bmagic |
it was many months ago, no one remembers |
| 12:31 |
berick |
Bmagic: that means those try/catch'es do nothing. that crash is happening in a whole different OS process |
| 12:32 |
berick |
(the api should also be fixed, of course, but on the topic of try/catching...) |
| 12:32 |
Bmagic |
we'll dig |
| 13:01 |
Dyrcona |
My other test turns out to have a bad template... |
| 13:07 |
Dyrcona |
Ah, found it. Though the error message didn't really help.... It complained about an unexpected token (END) when it looks like the problem was an extra %] higher up. |
| 13:57 |
|
jihpringle joined #evergreen |
| 14:29 |
Stompro |
Great... testing EDI is fun. Just sent in an order because "edi_pusher.pl --debug-only=1" is not the correct format for that flag, so it just runs like normal. |
| 14:30 |
jeffdavis |
:( |
| 14:35 |
Stompro |
I guess I'll get to see what happens when PO and line item id numbers get reused does to the B&T system and to our system if they send us a response message. |
| 14:37 |
Stompro |
The test seems to be showing that EDI generates the same message on 3.11.1, with Ruby 3.1 and the Ruby EDI translator on Debian 12 as it did on our old system. |
| 14:38 |
jeff |
later that same day, From: Baker & Taylor / Subject: Server Outage Impacting Systems |
| 14:44 |
Stompro |
Ha, but maybe it will fix the content cafe issue... if they have to restart their redis servers. Ever hopeful. |
| 14:52 |
|
jihpringle joined #evergreen |
| 14:58 |
Dyrcona |
Ah, the mysteries of Cronscript.pm.... |
| 15:01 |
Dyrcona |
I sometimes think the way it (ab)uses Getoplong should be changed, but then I'm likely the only person who uses it with much frequency. |
| 15:18 |
|
jihpringle69 joined #evergreen |
| 15:30 |
|
jihpringle joined #evergreen |
| 16:40 |
Bmagic |
what happened here https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/blake/lp_2012669_hold_reset_reason_main_branch |
| 16:41 |
Bmagic |
the group of commits at the top "LP#2012669 Hold reset reason" are exact repeats of the same group further down |
| 16:44 |
Bmagic |
oh well, I'm force committing the proper sequence |
| 16:52 |
Dyrcona |
Bmagic: Did you clean up the branch already? |
| 16:52 |
Bmagic |
yeah |
| 16:52 |
Bmagic |
it looked like old main plus patch, plus more main commits plus patch again |
| 16:55 |
Dyrcona |
The branch looks OK, now. |
| 16:55 |
* Dyrcona |
calls it a day. |
| 17:01 |
|
mmorgan left #evergreen |
| 17:39 |
|
BAMkubasa joined #evergreen |
| 21:52 |
|
sandbergja joined #evergreen |
| 21:52 |
|
sandbergja left #evergreen |
