| 15:19 |
redavis |
!!! |
| 15:19 |
Bmagic |
#link https://evergreen-ils.org/downloads/Evergreen-ILS-3.13-beta.tar.gz |
| 15:19 |
sandbergja |
nice! woohoo! |
| 15:20 |
Bmagic |
If anyone has some time, testing that would be sweet |
| 15:20 |
mmorgan |
Bmagic++ berick++ shulabear++ sleary++ abneiman++ |
| 15:20 |
dluch |
releaseteam++ |
| 15:20 |
redavis |
Bmagic++ berick++ shulabear++ sleary++ abneiman++ |
| 15:30 |
redavis |
That was my thinkin's. Cool. Thanks for the spell wisdom. Makes sense. |
| 15:30 |
Bmagic |
ok, that's another branch to merge..... |
| 15:30 |
Bmagic |
someone else should probably do it |
| 15:30 |
Dyrcona |
FWIW, I don't disable JIT on my test db with Pg12+. |
| 15:31 |
csharp_ |
I would have to watch a tutorial - my scant reading doesn't have me grokking it yet |
| 15:31 |
Bmagic |
(since I authored it) - anyone want to take the action item? |
| 15:31 |
csharp_ |
could be one of those things that tests fine but causes havoc at scale |
| 15:31 |
Bmagic |
or I guess I should back up and ask if it needs* to make it in before 3.13 is official |
| 15:32 |
csharp_ |
I think since PG11 is dead, we should push it asap |
| 15:32 |
Dyrcona |
csharp_: Depends on a number of factors from what I understand. |
| 15:40 |
csharp_ |
1 bug <--> 1 issue is the kind of granularity I try for |
| 15:40 |
mmorgan |
Commits can get missed. |
| 15:40 |
csharp_ |
but I'm not developing big ol' UIs and things |
| 15:40 |
kmlussier |
But if the commit with the shared component is added to a specific launchpad bug, and then somebody has feedback on that shared component as part of testing, won't that discussion be on that specific LP bug even if it's for a distinct feature? |
| 15:40 |
csharp_ |
mmorgan: I agree |
| 15:41 |
kmlussier |
csharp_: So are you saying those shared components should get their own distinct bug? |
| 15:41 |
redavis |
(s/omnibus bus (that's funny)/omnibus bug) |
| 07:59 |
|
BDorsey joined #evergreen |
| 08:18 |
|
kworstell-isl joined #evergreen |
| 08:34 |
|
mmorgan joined #evergreen |
| 08:45 |
mmorgan |
rlefaive: bug 1993824, released in 3.12 resolved those issues. I did a quick test on a test server and was able to download csv and delete a session. |
| 08:45 |
pinesol |
Launchpad bug 1993824 in Evergreen "wishlist: Angular Link Checker Interface" [Wishlist,Fix released] https://launchpad.net/bugs/1993824 |
| 08:50 |
|
Dyrcona joined #evergreen |
| 08:57 |
|
dguarrac joined #evergreen |
| 11:49 |
Dyrcona |
I think verify_passwd requires a hash of the password. |
| 11:51 |
csharp_ |
or the salt |
| 11:51 |
Bmagic |
looking at the function, maybe I can delete the row in actor.passwd? |
| 11:52 |
Dyrcona |
Bmagic: actor.change_password worked for me yesterday on a system that's running main from early last month. I changed a patron's password on the test system to play with NCIPServer authentication. |
| 11:52 |
Dyrcona |
I could login with the new password. |
| 11:53 |
Bmagic |
no go on my test machine. Tried srfsh also |
| 11:54 |
Bmagic |
srfsh# login admin demo123 |
| 11:56 |
Bmagic |
something is afoot |
| 11:56 |
Bmagic |
I'm sure it's me, crawling logs |
| 15:57 |
Dyrcona |
I don't think there can be one because acn doesn't have a field that links to actor.usr, but I could be wrong. |
| 15:58 |
jmurray-isl |
I was wondering. The template gets its user info from the user_data variable, instead. |
| 15:59 |
Dyrcona |
Yeah. Evergreen supplies the user data when the event is created. |
| 16:00 |
jmurray-isl |
In addition to that one, the Send Test SMS action trigger (hook: au.sms_text.test) doesn't seem to have context_usr_path defined either, but the primary target should be actor.usr.id. I've tried various things, but nothing has worked to populate that one, either. |
| 16:00 |
Dyrcona |
For that event, you only get the carrier and the number. |
| 16:02 |
Dyrcona |
My previous comment was for texting a call number. |
| 16:02 |
Dyrcona |
For au.sms_text.test the core type is au. |
| 16:02 |
jmurray-isl |
Yep, I've tried, target, target.id, and id and none seem to work. |
| 16:03 |
Dyrcona |
What's your ultimate goal? |
| 16:04 |
jmurray-isl |
To populate context_user on SMS action trigger events so I can reference them in an SQL query daily report on the counts of SMS events to various carriers |
| 16:05 |
jmurray-isl |
Those two action trigger definitions were the outliers. |
| 16:06 |
jmurray-isl |
Their number is minimal, to be fair, but I was mostly wondering for completion sake. |
| 16:09 |
Dyrcona |
For the send test sms, you can use the target in your SQL. |
| 16:10 |
jmurray-isl |
Yep. |
| 16:11 |
Dyrcona |
I don't think you can get anything in the text call number event context_usr without changing Perl code. |
| 16:11 |
jmurray-isl |
Gotcha. |
| 16:19 |
JBoyer |
FYI for GitLab hosters, they’ve laid a security egg re: password resets: https://arstechnica.com/?p=2021409 |
| 16:19 |
JBoyer |
Update asap |
| 16:19 |
|
jihpringle joined #evergreen |
| 16:23 |
Dyrcona |
jmurray-isl: Yes. It's pulled from the target using the context_usr_path. Since the usr is the target for the test sms event, it's not possible to grab it. Look at OpenILS/Application/Trigger/Event.pm around line 496. |
| 16:25 |
Dyrcona |
jmurray-isl: When you tried id on the send test sms, did you restart the open-ils.trigger service after making the change? "id" might work on that one since the core_type is au. |
| 16:27 |
jmurray-isl |
I did not restart the service. I'll try that. |
| 16:30 |
Dyrcona |
I was trying to test it, too, but I didn't set websockets port last time that I built opensrf for debuggin. |
| 16:34 |
Dyrcona |
then a hard reload in Chrome... |
| 16:38 |
Dyrcona |
Context user was filled in with the wrong id, and the event errored. |
| 16:40 |
Dyrcona |
Nope. I take that first part back. context_user was not set. The event did error. |
| 12:18 |
dickreckard |
Bmagic: a new clean database works correctly with the current Evergreen bits. it seems to be the upgraded db that breaks the keyword search ( and probably other things then. ) |
| 12:18 |
Bmagic |
I see what you mean, carry on |
| 12:26 |
|
smayo joined #evergreen |
| 12:49 |
Dyrcona |
I have test/dev servers where features sometimes don't work. I've got one where the acq. interface completely fails. Search has been a problem in the past. A full ingest sometimes helps. |
| 12:49 |
Dyrcona |
dickreckard ^^ |
| 12:57 |
Dyrcona |
We usually blame these issues on the test environment being slower than production. |
| 14:05 |
|
sleary joined #evergreen |
| 14:54 |
Dyrcona |
oops. accidentally pushed a working branch to NCIPServer origin, and gitolite won't let me delete it. I should probably check/update the configuration to prevent pushing working branches in that repo. There's only 3 of us that use it, anyway.... |
| 15:09 |
|
kworstell-isl joined #evergreen |
| 16:06 |
Dyrcona |
I guess a username that's 14 digits will cause us problems.... I've recently come out against letting patrons chose their own usernames in light of these issues and other bugs. |
| 16:07 |
Dyrcona |
I've been outvoted, so I go along with it. |
| 16:08 |
Dyrcona |
Stompro: Do you want to update the code or should I? |
| 16:09 |
Stompro |
Feel free, I'll hopefully be able to test it out tomorrow afternoon. I don't get why usernames don't actually work either, that might be a different bug. |
| 16:10 |
Dyrcona |
Well my barcode == username, and breaks the rules. I should have tested with a regular patron, and in light of this, I will test it again. |
| 16:11 |
Dyrcona |
I think I know the account to test with. One of those with the lowercase d in the barcode. They also have a different username. |
| 16:13 |
Dyrcona |
Hm.. the lowercase d password is inactive in my test database and the uppercase D one is active. I guess that's even better. :) |
| 16:17 |
Dyrcona |
Also, the code was implemented to only expect barcodes, but.... |
| 16:21 |
Stompro |
do you mean the "<AuthenticationInputType>barcode</AuthenticationInputType>" part? |
| 16:22 |
Stompro |
I noticed that also, but I don't remember what the other options in the standard were there off the top of my head. |
| 12:00 |
|
jihpringle joined #evergreen |
| 12:10 |
Bmagic |
Dyrcona: I'm troubleshooting the SFTP ingram sftp thing. I've successfully logged into their server using a GUI tool, but the funny thing is, their server is sending my GUI tool a banner message. Which, I think, is tricking the Evergreen code. We're able to push orders, but we can't receive. When attempting to scp manually via scp command, I get this: "exec request failed on channel 0" - which is their server attempting to use my tty ( |
| 12:10 |
Bmagic |
according to google) |
| 12:11 |
Bmagic |
did you deal with this during testing? |
| 12:11 |
berick |
hm, just deployed locally. i've only testing sending so far. |
| 12:11 |
berick |
*tested |
| 12:13 |
Bmagic |
bouncing this off of you: scp account_redacted ftp.ingramcontent.com:/outgoing/filename_redacted.EIN ./test.ein |
| 12:13 |
Bmagic |
does that look sound? |
| 12:14 |
Bmagic |
(it prompts for the password) - which I provide, then I get the "exec request failed on channel 0" error |
| 12:16 |
berick |
Bmagic: that fails for me too, but using "sftp" instead of "scp" works OK |
| 15:20 |
|
kworstell-isl joined #evergreen |
| 16:21 |
Stompro |
I think the fake ISBN's that B&T creates for DVDs are not conflicting with real codes. Looks like Romania and Argentina were assigned 630 and 631 in 2023. |
| 16:21 |
Stompro |
s/are not/are now/ |
| 16:36 |
Stompro |
But, a test system with ISBN::Data from 2021 also doesn't load them... so maybe something else is causing this. |
| 16:39 |
Dyrcona |
Fake ISBNs were problem never a good idea. |
| 16:40 |
Dyrcona |
s/problem/probably/ but somehow problem is more appropriate though is not an adverb. |
| 16:50 |
mmorgan |
probably a problem |
| 15:37 |
Bmagic |
I think* it was saving a bib record, which in-turn queried reporter.old_super_simple_record - which is what's written on the bug report |
| 15:37 |
eeevil |
there are certain things that JIT wants to do that we have already poured a lot of brain power into manually tuning |
| 15:37 |
Bmagic |
that makes sense |
| 15:38 |
eeevil |
it's not that EG is incompatible with PG's JIT, it's that (just like autovac and stats targets, and lots of other stuff, tbh) it requires config that is tuned to your particular size and shape of data |
| 15:39 |
eeevil |
a small EG instance may benefit a BUNCH from JIT, and reporting might as well (I've tested neither, but I have suspicions) |
| 15:40 |
sandbergja |
thanks, eeevil, that's helpful info to have |
| 15:40 |
eeevil |
but(!) unless someone has the tuits to learn the feature well, and analyze our queries in that context, I agree that we should recommend disabling JIT for now |
| 15:40 |
Bmagic |
eeevil++ |
| 15:41 |
eeevil |
I'd actually propose we investigate (as a community, as broadly as we can manage) cross-column stats targets before jumping into JIT |
| 15:41 |
Dyrcona |
I'm OK with that, but I don't think I've every encountered an issue caused by JIT. Pg 16 (even unoptimized) has pretty much always been faster than Pg 10 for me. BUT! I've not run it in produciton, just in testing on a production-sized database with a production-sized server. |
| 15:41 |
eeevil |
(not to derail the JIT-specific discussion, but as a query tuning tool I think that will pay off for more folks) |
| 15:42 |
eeevil |
Dyrcona: I haven't seen JIT-specific issues either, but I think we can be fairly sure that plan stability is /higher/ without JIT (until we learn to tune it well) |
| 15:43 |
Bmagic |
Disabling it fixed our problem on a test-then-production instance |
| 15:43 |
Bmagic |
pretty sure it manifested as a time-out when clicking on save in the MARC editor |
| 15:43 |
Dyrcona |
So, we'll recommend disabling JIT in the installation instructions, then. |
| 15:43 |
eeevil |
which is to say, yes, some folks will lose out on some optimizations they get "for free" with JIT on, but things are much less likely to go sideways in ways some have seen |
| 07:53 |
|
collum joined #evergreen |
| 07:58 |
|
BDorsey joined #evergreen |
| 08:36 |
|
Dyrcona joined #evergreen |
| 08:42 |
Dyrcona |
Guess, I'll undo my testing changes on the test database and try the fetcher with the production settings and grab the files from Ingram's server. Maybe there's something that my test changes are masking. |
| 08:43 |
Dyrcona |
I did add vsftp on my sftp host, and configured one account to get files with ftp and others with sftp and that did not seem to cause files to be downloaded more than once in my test environment. I thought that using ftp and sftp with basically the same account would do it, but it didn't. |
| 08:46 |
Dyrcona |
I have also verified that FTP is working. |
| 09:03 |
Dyrcona |
Feels like there should be a way to do this with pg_dump/pg_restore, but so far no dice. I think I'll try a data-only dump with --disable-triggers and a truncate on the tables after. |
| 09:11 |
Dyrcona |
pg_dump: warning: there are circular foreign-key constraints among these tables: |
| 10:12 |
berick |
been meaning to move in that direction myself |
| 10:13 |
Dyrcona |
OK. I guess we agree more than I thought yesterday. |
| 10:13 |
berick |
also considering keeping local copies of the plain files -- before they go into the DB -- as a secondary backup for debugging, etc. |
| 10:13 |
Dyrcona |
I still haven't been able to replicate what I saw in production on my test environment, so I'm trying to restore the original settings. I'm running the fetcher to get caught up. |
| 10:14 |
berick |
possibly a lighter way to say "we already have this" than querying the DB |
| 10:14 |
Dyrcona |
Yeah, that is worth exploring. |
| 10:15 |
Dyrcona |
Back to the issue, I thought it must have to do with 1 account having sftp in the host and the others having ftp with the same credentials and directory, but even when I did that on my test setup, it didn't pull the files twice. |
| 10:16 |
Dyrcona |
This other account says X of 1722 targets.... |
| 10:34 |
|
jvwoolf joined #evergreen |
| 10:45 |
Dyrcona |
Ok. I've got only the Ingram accounts set to active. I did a run to get caught up and grabbed 63 files. The second run grabbed 0. Now, I'm going to set 1 account to sftp in the host and see what happens. There are other accounts with the same credentials using FTP. This should replicate what we saw in production. |
| 10:48 |
|
sandbergja joined #evergreen |
| 10:54 |
Dyrcona |
Files retrieved: 275 |
| 10:55 |
Dyrcona |
Wow! I think it is the host part of the duplicate check in retrieve_core after all. Dunno why it didn't trigger on my local test server, though. |
| 10:56 |
Dyrcona |
berick: I think we should regexp_replace('^[a-z]+://', host, '') in the host check, or we just need to update all of the same accounts at once? |
| 10:57 |
Dyrcona |
or, maybe [^:]+ instead of [a-z]+? |
| 10:58 |
Dyrcona |
I'm running the same again to see what happens. I suspect it should grab 275 again unless a new file has gone up. |
| 11:45 |
Dyrcona |
csharp_: We had something local that was written in PHP. Now, we're using Metabase. |
| 11:45 |
Dyrcona |
It's not built into Evergreen, though you could add links. |
| 11:46 |
Dyrcona |
So, setting all of the accounts with the same username and password to use sftp resolves the multiple files retrieval, as I thought it might. |
| 11:48 |
Dyrcona |
Moral of the story: Don't just set 1 account to SFTP to "test" it. It's all or nothing. |
| 11:51 |
jeff |
and take steps to ensure that nothing tries to fetch files while you make the config changes to all the accounts in question? |
| 11:52 |
Dyrcona |
jeff: A SQL update should be fast enough and much faster than doing it in through the client. |
| 11:54 |
Dyrcona |
I may suggest a small path to O::A::Acq::EDI.pm to remove the scheme from the URL stored in host field of acq.edi_account. Doing a schemeless comparison would also resolve the issue. |
| 15:40 |
StomproJ |
Yes, for a number of years. |
| 15:40 |
Bmagic |
StomproJ++ |
| 15:40 |
Bmagic |
we gotta get this merged then |
| 15:41 |
StomproJ |
Works for SMS call number also, and for the SMS test message.... although there is a bug there I need to document. |
| 15:42 |
Bmagic |
I'm reading https://gitlab.com/LARL/evergreen-larl/-/commit/73b424779e2a07e351cc5cd78c8c2ee32b12fd6e |
| 15:43 |
StomproJ |
Testing is probably hard since you need a flowroute account setup with real money involved. Plus all the SMS Campaign registration stuff. |
| 15:44 |
Bmagic |
right, so, it'll never get merged.... how do we "trump" that? |
| 15:44 |
Dyrcona |
From what I understand, the campaign registration is pretty much required if you want your messages to get through, even with the commercial gateway. |
| 15:44 |
StomproJ |
Oh, I need to update it for the latest version of their API also, which improves the delivery report data. |
| 15:45 |
Dyrcona |
Bmagic: You've got the power.... |
| 15:45 |
Bmagic |
the branch needs documentation and probably a pg test and a perl test |
| 15:46 |
Dyrcona |
EDI is also hard to test. |
| 15:47 |
StomproJ |
I could probably provide api keys for someone to test... I'll have to think about how to do that. |
| 15:48 |
Bmagic |
StomproJ++ |
| 15:48 |
Dyrcona |
I wouldn't want anyone to get in trouble for sharing test API keys. |
| 15:48 |
Bmagic |
maybe we could create a trial account for testing |
| 15:49 |
StomproJ |
I'll reach out to Flowroute and see if they can provide trial accounts. |
| 15:50 |
Bmagic |
Flowroute might get an increase in business, which makes it feel "icky" to only support them, but we gotta start somewhere |
| 15:50 |
Dyrcona |
There's precedent with stripe. Someone with an account has to test it. |
| 15:51 |
Dyrcona |
Well, that's the code that there is, supplied by a community member. I don't see anyone else sharing anything. (Says the guy who's not a fan of commercial solutions in general, but in this case, what can you do?) |
| 15:55 |
Bmagic |
I prefer Evergreen to talk to a commercial SMS API, rather than generate XML and ship that to MessageBee (for example) |
| 15:56 |
Bmagic |
Shipping XML and maintaining a bunch of AT template toolkit's is more annoying IMO |
| 15:59 |
StomproJ |
I'm using a php script to just send them to my email now. You just tell flowroute your callback url. |
| 16:00 |
Dyrcona |
"Click here to spam the world" |
| 16:00 |
Bmagic |
What's the next step to get this Flowroute code merged? We have a bug, and the code, though the code isn't in the "normal" form/branch method? A signoff maybe? It could use the documentation pieces (release notes, and proper doc injections). I'm sure everyone would welcome such an addition, and the SMS fire has been burning for a long time now |
| 16:01 |
Dyrcona |
Bmagic: You just said it. It needs signoff and release notes minimum. Tests and documentation would be most welcome. |
| 16:02 |
Bmagic |
Seems pretty doable to get it over the finish line IMO. StomproJ: maybe we can divide the tasks? |
| 16:04 |
Bmagic |
I think it would be "easier" (for me) if it were in the working repo |
| 16:04 |
StomproJ |
Sure, I think just having someone else interested in it helps... Probably docs on setting up a flowroute account also... |
| 16:17 |
Bmagic |
sweet, StomproJ: you good with rolling with the collab branch? |
| 16:17 |
StomproJ |
Sure, sounds good. |
| 16:17 |
Dyrcona |
Bmagic++ StomproJ++ |
| 16:18 |
Bmagic |
Sounded like you were interested in the documentation additions, and release notes? I can try my hand a tthe tests |
| 16:18 |
StomproJ |
Here are all the flowroute related commits that I have in our custom production repo. https://gitlab.com/LARL/evergreen-larl/-/commits/rel_3_11_1-larl?search=flowroute |
| 16:19 |
Bmagic |
StomproJ: I think/hope those are the exact commits I just put onto our collab branch |
| 16:19 |
Dyrcona |
https://gitlab.com/LARL/evergreen-larl/-/commit/3d4cdbc7d6260640e5f086c2965dccc3797e27d1 isn't in the collab branch, and I wonder if it is necessary. |
| 13:32 |
berick |
Dyrcona: i'd stop short of deleting the files after download since that's new behavior, but fixing the dupe detection is obv. critical. |
| 13:33 |
Dyrcona |
In the long run, I think we should delete the files, or make it an option. As far as I can tell, Ingram isn't cleaning up the outgoing directory. I have not checked other vendors, yet. |
| 13:34 |
Dyrcona |
So, I'll agree with that not being a bug fix. |
| 13:36 |
Dyrcona |
Meanwhile, I can test with local servers and messages from production. |
| 13:36 |
Dyrcona |
Wonder if the fetcher works with the debugger? |
| 13:47 |
Dyrcona |
I swear that I saw the duplicate detection code last week, but can I find it right now? Nope. |
| 13:48 |
Dyrcona |
It's probably in OpenILS::Application::Acq::EDI->process_retrieval |
| 15:26 |
|
jvwoolf joined #evergreen |
| 16:03 |
|
sandbergja joined #evergreen |
| 16:11 |
|
jvwoolf joined #evergreen |
| 16:30 |
Dyrcona |
OK. I cannot reproduce the duplicate retrieval in any of my 3 test environments with recent files. |
| 16:31 |
Dyrcona |
This requires further investigation. |
| 16:52 |
Dyrcona |
Well, that's that fo today. |
| 16:59 |
|
Guest21 joined #evergreen |
| 09:55 |
Dyrcona |
I think you do have to specify the remote though when pushing a new branch, if you didn't already set it to track something. |
| 10:11 |
|
mmorgan joined #evergreen |
| 11:49 |
|
jihpringle joined #evergreen |
| 12:14 |
Dyrcona |
Odd. In production, the SFTP code seems to be setting the remote filename to "1" while on my test vm it set it to the actual remote filename. I've compared RemoteAccount.pm between production and the test system, and they are identical. |
| 12:14 |
Dyrcona |
Also both are on the same release of Ubuntu with the same Perl version, and the same Net::SFTP::Foreign module version. |
| 12:16 |
Dyrcona |
I wonder if it is possibly a difference in the server? Could the SFTP server affect the return value of $sftp->put()? |
| 12:21 |
Dyrcona |
Has anyone else started using/testing SFTP with Ingram in production? If so, have you noticed anything in the acq.edi_message table, notably remote_file having just a "1" in it? |
| 12:41 |
Dyrcona |
i think there's a bug in the put_sftp function that somehow escaped testing. I unfortunately do not have syslogs on the test machine going back far enough to see what happened there. I'm going to run another test. |
| 12:50 |
Dyrcona |
Actually, I think I see the bug on one of my test systems. I'm going to test again and see what happens. |
| 13:05 |
|
sleary joined #evergreen |
| 13:44 |
Dyrcona |
Bleh. None of the production data is loading today. Too much missing extra data: bibs, etc. |
| 13:46 |
Dyrcona |
When I tried to add items via brief record, I get a failure to update the database. As I said, I don't know what I'm doing in the acq interface. |
| 14:07 |
* Dyrcona |
reloads a database dump or two.... |
| 14:11 |
|
collum joined #evergreen |
| 14:12 |
Dyrcona |
I suppose I could just wait until next week, but I really want to verify this bug. |
| 14:16 |
Dyrcona |
Found it in 1 account on a database that I have not reloaded, yet! remote_file is 1 for one of the purchase orders that I successfully tested. |
| 14:20 |
Dyrcona |
So, I'm going to test on one of the vms with the patch applied to see if that fixes it. |
| 14:20 |
Dyrcona |
But, first, the database must finish reloading. |
| 14:32 |
Dyrcona |
Lp 2060153 |
| 14:32 |
pinesol |
Launchpad bug 2060153 in Evergreen 3.12 "EDI SFTP Records wrong remote file when sending purchase order " [Undecided,New] https://launchpad.net/bugs/2060153 |
| 14:45 |
Dyrcona |
audit_time has fractions of a second and order_date does not. |
| 14:52 |
|
stephengwills joined #evergreen |
| 14:58 |
jeff |
yeah, timestamps that are set by DEFAULT NOW() in the database will go backward in time when those rows are later updated by... just about everything else in Evergreen. |
| 15:00 |
Dyrcona |
It makes me wonder how I got any data to test last time, though. I should check the auditor cleanup for that table. |
| 15:15 |
|
sleary_ joined #evergreen |
| 15:18 |
Dyrcona |
Ah. I see. The purchase order was updated in produciton since I last grabbed data, so the timestamps lost some precision. |
| 15:18 |
Dyrcona |
s/produciton/production/ |
| 10:28 |
|
JBoyer joined #evergreen |
| 11:09 |
Bmagic |
JBoyer: Dyrcona: I have the branch staged locally (on main) - would you mind reviewing my git situation before* I push? https://pastebin.com/tsBk2LrG |
| 11:10 |
Bmagic |
I included the last commit on remote main at the bottom of that paste for context |
| 11:18 |
Bmagic |
I tested the branch as far as confirming that Evergreen will build after the patch is applied to main (and concerto installs fresh) |
| 11:28 |
Dyrcona |
Bmagic: It looks allright, but don't use those commit messages if you put that into main. ;) |
| 11:29 |
Dyrcona |
Bmagic: Do you have a way to test the EDI transfer? |
| 11:29 |
Bmagic |
I'm sure you noticed that the commit messages are the combined blocks from the squashed commits for each author |
| 11:29 |
Bmagic |
testing EDI transfer: no |
| 11:30 |
Dyrcona |
Yeah, I noticed. I don't mind the way you squashed them, but I'd edit the commit messages. |
| 11:30 |
Dyrcona |
I used another vm and modified a couple of EDI accounts. I had a 1,000 word essay on it but Lp rejected the comment. :) |
| 11:31 |
Bmagic |
Like changing the wording? Maybe you could provide your suggested changes in a pastebin? |
| 11:32 |
berick |
safe to assume Ingram already has SFTP running on their side? |
| 11:32 |
Dyrcona |
I'm in a local meeting where we earlier discussed setting it up in production next week and testing with 1 library that uses Ingram. |
| 11:34 |
Dyrcona |
You could test it locally with another user account on the same vm. |
| 11:34 |
Bmagic |
hmm, that's an interesting idea |
| 11:35 |
Bmagic |
I do have the code currently installed and propped up on a local machine... I think you're saying I could create a linux user on the same machine and stage a PO to post to that user's home folder via ssh |
| 11:40 |
berick |
ok, confirmed the existing Ingram FTP host also supports sftp connections |
| 13:09 |
|
mantis left #evergreen |
| 13:23 |
* Dyrcona |
is about to switch networks. |
| 13:25 |
|
Dyrcona joined #evergreen |
| 13:25 |
Bmagic |
I got it tested yall! Successfully sent EDI message to: sftp://localhost |
| 13:26 |
Dyrcona |
Bmagic++ |
| 13:26 |
Bmagic |
I'm going to remove the patch and make sure it doesn't* work without it |
| 13:26 |
Dyrcona |
The tricky part was getting it to pick up messages. |
| 13:27 |
Bmagic |
yeah, that'd require hand-crafting the response EDI |
| 13:27 |
Bmagic |
if it can push, it ought to be able to pull right? |
| 13:27 |
Dyrcona |
That's why I used production data on a test system that was a bit behind. |
| 13:28 |
Dyrcona |
You can pull the EDI out of acq.edi_message. |
| 13:29 |
Dyrcona |
Bmagic: The code to pull (get) is different and it didn't work until yesterday afternoon. |
| 13:31 |
Bmagic |
so the return EDI message can just be the same as the push EDI message? |
| 13:34 |
Dyrcona |
I seem to recall it not working for me years ago. csharp_ might be able to better explain the issues. |
| 13:37 |
Bmagic |
I'm verifiying that the patch is actually not* on my machine. I'm confirming that by looking at some of the code changes in the git commit(s) and seeing that they are not in the file in /usr/local/share/perl/5.34.0/OpenILS/Utils/RemoteAccount.pm and they aren't |
| 13:37 |
Bmagic |
so, I'll double check and restart Evergreen services and try again |
| 13:37 |
Dyrcona |
Sure. I'm not contradicting you. It may not work under some circumstances. Also, you should try to test getting a message off the remote. |
| 13:41 |
Dyrcona |
Full disclosure: I did not test with the pre-existing code this time around because a) I took the bug report at face value and b) I recall trying it years ago at MVLC on a test system and something didn't work right. (I don't remember the exact details.) |
| 13:44 |
Bmagic |
well, I'm pretty sure it pushes the EDI message without the patch, I'll check pull |
| 13:44 |
Dyrcona |
Bmagic: How is your EDI account setup? Do you have separate directories for incoming and outgoing files? |
| 13:45 |
Bmagic |
yeah |
| 14:05 |
Dyrcona |
True, but if it ain't broke, don't fix it. :) |
| 14:05 |
|
Guest2747 joined #evergreen |
| 14:07 |
Dyrcona |
It might be worth asking csharp__ to elaborate on the uses cases where the current code doesn't work. |
| 14:10 |
Dyrcona |
I also see something else that should be tweaked in the branch: StrictHostKeychecking ought to be StrictHostKeyChecking, but I have verified that the former works by testing from a vm with an account that never connected to the SFTP vm before. |
| 14:11 |
* Dyrcona |
wonders if that might have been the main issue all along, not having the remote host key in ~opensrf/.ssh/known_hosts. |
| 14:14 |
Bmagic |
hmmm |
| 14:17 |
mmorgan |
Bmagic: Dyrcona: This is from an Ingram service alert message "In October, we alerted you that as of April 15, 2024 we would no longer support non-secured FTP connections and that all existing and future FTP accounts would have to use Secure FTP (sFTP)." |
| 14:18 |
Dyrcona |
mmorgan: Yeah, we're aware of the deadline. Right now, the question is does the current implementation of SFTP in OpenILS/Utils/RemoteAccount.pm need to be replaced. |
| 14:18 |
mmorgan |
Ok, gotcha. |
| 14:18 |
Dyrcona |
I'm going to revert one of my two test systems and try what was in 3.7.4. |
| 14:27 |
Dyrcona |
I found an invoice I can dump from production and use as a test. |
| 14:32 |
Dyrcona |
Bmagic: I just did a test with an invoice and the current implementation did not pick it up. I removed the host key from ~opensrf/.ssh/known_hosts. I'll add it and see what happens. |
| 14:34 |
Dyrcona |
Nope. still says: Files retrieved: 0 |
| 14:38 |
Dyrcona |
Bmagic: On 3.7.4 on my test set up, the stock Evergreen RemoteAccount.pm failed to retrieve the file. With the Lp 2040514 branch applied, it worked, even without the key in ~opensrf/.ssh/known_hosts. |
| 14:38 |
pinesol |
Launchpad bug 2040514 in Evergreen 3.12 "EDI SFTP doesn't work" [High,Confirmed] https://launchpad.net/bugs/2040514 |
| 14:39 |
Bmagic |
ok then, I must have a bad test |
| 14:39 |
Dyrcona |
You might just have conditions where the current code works. Also, it could be more broken in older Evergreen. I'm going to try the same test again on main. |
| 14:40 |
Bmagic |
I'm using main as my control test |
| 14:40 |
* Dyrcona |
has another vm almost all set to go. |
| 14:41 |
Dyrcona |
All you have to do to test one implementation versus the other is swap out /usr/local/share/perl/5.34.0/OpenILS/Utils/RemoteAccount.pm. The 5.34.0 part will be different depending on your Perl version. |
| 14:50 |
Dyrcona |
Looks like the current implementation in main is working. I wonder what changed (if anything) and when? I wonder if it has something to do with Linux release and/or Perl module versions? |
| 14:50 |
Bmagic |
oh? |
| 14:51 |
Bmagic |
so, I don't have a broken test? |
| 14:51 |
Dyrcona |
My main test vm is Ubuntu 22.04 and the other is Ubuntu 20.04. |
| 14:51 |
Bmagic |
yep, 22.04 and 5.34.0 perl for my testing |
| 14:51 |
Bmagic |
current main is both pushing and pulling via sftp://localhost (sans patch) |
| 14:51 |
Dyrcona |
It did not work for me on 20.04 with Perl 5.30. |
| 14:52 |
Bmagic |
interesting |
| 14:52 |
Dyrcona |
I only tested pull. |
| 14:52 |
Dyrcona |
I'm going to test pull again on 20.04. I've added another file. |
| 14:54 |
Dyrcona |
Yeah, stock implementation definitely not working there with Evergreen 3.7.4. I don't want to upgrade that one to main right now. |
| 14:55 |
Dyrcona |
I wonder if it works on Bullseye or Bookworm? |
| 14:57 |
Bmagic |
if the patch makes it work for more/various installations of Evergreen on different platforms, then, I think we should merge |
