Time |
Nick |
Message |
00:43 |
|
sleary joined #evergreen |
02:00 |
|
eby joined #evergreen |
04:11 |
|
dickreckard joined #evergreen |
07:14 |
|
collum joined #evergreen |
08:04 |
|
BDorsey joined #evergreen |
08:26 |
|
redavis joined #evergreen |
08:40 |
|
mmorgan joined #evergreen |
08:47 |
|
dguarrac joined #evergreen |
09:17 |
|
mantis joined #evergreen |
09:32 |
|
kmlussier joined #evergreen |
09:57 |
|
sleary joined #evergreen |
10:09 |
|
Dyrcona joined #evergreen |
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 |
11:41 |
|
sandbergja joined #evergreen |
11:46 |
|
stephengwills joined #evergreen |
11:54 |
Dyrcona |
Bmagic: Yes, that's what i'm saying. I had another vm but the principle is the same. I made 2 directories: 'incoming' and 'outgoing' to mimic Ingram. |
12:05 |
|
jihpringle joined #evergreen |
12:12 |
JBoyer |
Bmagic, I'd also suggest editing the commit messages to remove the extra LP1234: lines in the middles at least. And I'm not sure what command you're running, but I assume the commit from Ken about Stripe was just where you started working on this branch? |
12:13 |
Bmagic |
yep, see my comment above about the bottom commit was included in my pastebin for context on main |
12:13 |
Bmagic |
and yes, I agree about removing the redundant lines with LP bug number in the squashed commit header lines, though I debated that when I was squashing, ultimately decided to leave them because they were part of the authors info |
12:15 |
Bmagic |
but it sounded like Dyrcona was suggesting additional edits to the commit messages |
12:16 |
|
Dyrcona joined #evergreen |
12:17 |
Dyrcona |
I'm in meetings. I can suggest edits after. |
12:33 |
Dyrcona |
Bmagic: I'd actually prefer not to have my two functional commits squashed. I think adding the stricthostchecking option should be separate so it can be easily removed. |
12:37 |
|
collum joined #evergreen |
12:54 |
Bmagic |
Dyrcona: understood |
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:31 |
Dyrcona |
Well, it you rename it the fetcher will retrieve it, but it will not get processed into Evergreen. |
13:31 |
Dyrcona |
s/it/if/ |
13:32 |
|
sleary_ joined #evergreen |
13:32 |
Bmagic |
hmmm, without the patch, it still works |
13:33 |
Bmagic |
(push) |
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 |
13:45 |
Bmagic |
"incoming" "outgoing" without slashes on either end or file specification extensions |
13:45 |
Bmagic |
which lands the file in /home/ingram/incoming |
13:45 |
Dyrcona |
You can probably just put the EPO in the outgoing directory and run the fetcher and see what the output says. |
13:45 |
Bmagic |
so, now, I've done this: "cp /home/ingram/incoming/ygji86rYIB /home/ingram/outgoing/" |
13:47 |
Dyrcona |
The response usually gets .EPR and invoices .EIN. |
13:49 |
Bmagic |
mv /home/ingram/outgoing/ygji86rYIB /home/ingram/outgoing/ygji86rYIB.EIN |
13:50 |
Bmagic |
though, the config should tolerate any* filename as I've configed the EDI Account with "outgoing" |
13:53 |
Dyrcona |
You don't configure incoming globs usually. Ingram uses .EPR and .EIN. I think most vendors do. |
14:02 |
Bmagic |
well jees, it's working both pushing and pulling without the patch |
14:04 |
Dyrcona |
Well, you can always comment to that effect on the bug and ask why we need this code. I just went along with it. :) |
14:04 |
Bmagic |
on the other hand: it works with* the patch too.... |
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 |
14:57 |
Dyrcona |
I concur. |
15:01 |
Dyrcona |
Mar 28 14:54:06 egtest /openils/bin/edi_fetcher.pl: [WARN:175465:RemoteAccount.pm:374:] SSH2 connect FAILED: Resource temporarily unavailable -5 LIBSSH2_ERROR_KEX_FAILURE Unable to exchange encryption keys |
15:02 |
Dyrcona |
Found that in the logs on the Ubuntu 20.04 machine. The other host's key is in ~opensrf/.ssh/known_hosts, but I don't think libssh2 uses it by default. |
15:09 |
|
BDorsey joined #evergreen |
15:10 |
|
Guest2747 left #evergreen |
15:12 |
Dyrcona |
Bmagic: I force-pushed my collab branch 1 more time to clean up the option and a commit message. :/ |
15:27 |
Bmagic |
alrighty |
15:27 |
Bmagic |
sounds like I'm pushing? |
15:28 |
Dyrcona |
If you think it is ready to go. I don't think there's a need to squash the commits. As I said, I think the StrictHostKeyChecking thing should be its own so it's easy to revert if someone wants. |
15:29 |
Bmagic |
I had the notion of squashing for a little house-cleaning on main, but it's not a big deal to me. I was thinking that was good etiquette |
15:30 |
Dyrcona |
Yes, it can be. I think JBoyer's commits could be squashed. You could squash the release notes into the previous commit if you want. You want me to do that? |
15:31 |
Dyrcona |
The key checking change affects security, so it should be easily undone in my opinion. |
15:32 |
Dyrcona |
Also, I like how my Bluetooth mouse is turned off but it is still working. Must have a closed circuit somewhere. |
15:32 |
Dyrcona |
s/closed/open/ |
15:32 |
Bmagic |
if you want, maybe you can force push the collab branch exactly the way we want it on main |
15:33 |
Bmagic |
squashing JBoyer's commits, and yours as you see fit (and dressing up the messages at the same time) |
15:34 |
Dyrcona |
OK. I'll do that. I'll give the release notes another once over. I banged all of that out in a bit of a hurry yesterday afternoon. |
15:36 |
Bmagic |
ok great |
15:36 |
Bmagic |
happy to use my commit powers for (almost) the first time |
15:36 |
Dyrcona |
:) Don't forget the -s option to add your signoff! |
15:37 |
Bmagic |
glad you said! |
15:44 |
Dyrcona |
Bmagic: I think I'm done. It's up to you now. :) |
16:03 |
Bmagic |
alrighty |
16:11 |
Bmagic |
Dyrcona JBoyer: ok I pushed! |
16:11 |
Bmagic |
this goes to 3.12 as well I suppose? |
16:11 |
Bmagic |
What's the git command to just mirror the last 4 commits from one branch onto current branch? |
16:13 |
Dyrcona |
Bmagic: git cherry-pick -s ^main working/collab/dyrcona/lp2040514_reimplement_acq_edi_sftp_signoff # Will do it in this case. |
16:13 |
Dyrcona |
Assuming your main is up to date. |
16:13 |
Bmagic |
I already pushed main |
16:14 |
Bmagic |
oh, I think I see your command does what I want |
16:14 |
Dyrcona |
Not if the commits are in main, I don't think, but see what happens. |
16:14 |
Bmagic |
^main..main~4 is closer to what I'm trying to work out |
16:15 |
JBoyer |
Bmagic++ |
16:15 |
Dyrcona |
With .. you don't need ^. |
16:15 |
Dyrcona |
.. excludes the first. ... is inclusive. |
16:15 |
JBoyer |
it doesn't matter a lot in this case given the number of eyes on things, but it looks like you only signed off on the last commit. Can't fix that on main, but FYI. |
16:16 |
Bmagic |
I meant to sign off on the last commit. I didn't realize I needed to do it all the way down. But I know now! |
16:16 |
JBoyer |
Bmagic++ |
16:17 |
Dyrcona |
Bmagic++ |
16:17 |
Bmagic |
!! thanks yall |
16:17 |
Bmagic |
This does belong on 3.12 right? |
16:17 |
mmorgan |
Bmagic++ |
16:17 |
Dyrcona |
Yes, and 3.11 too |
16:17 |
mmorgan |
Dyrcona++ |
16:17 |
Bmagic |
git checkout rel_3_12 && git pull && git cherry-pick main...main~4 |
16:17 |
Bmagic |
that actually did what I think I wanted |
16:18 |
Dyrcona |
Yeah, looks like it would do what you wanted. |
16:19 |
Bmagic |
easy! Pushing to rel_3_12 and rel_3_11 unless there are objections? |
16:19 |
Dyrcona |
`git log main...main~4 ` shows only the commits you wanted |
16:19 |
Dyrcona |
I don't object. |
16:21 |
Bmagic |
merge conflict on 3.11 (figures) |
16:21 |
Bmagic |
Open-ILS/src/extras/install/Makefile.debian-bookworm doesn't exist |
16:22 |
Dyrcona |
Bmagic: I can take care of 3.11 later. |
16:22 |
Bmagic |
ok then, I'm done working on it then |
16:23 |
Dyrcona |
The change from that missing file will need to be added to buster or whatever was current in 3.11. |
16:24 |
* Dyrcona |
is about to head home but should be able to get it tonight. |
16:25 |
Bmagic |
much appreciated |
16:25 |
Bmagic |
Dyrcona++ JBoyer++ |
16:25 |
pinesol |
News from commits: LP#2040514: Net::SFTP::Foreign is not Net::FTP <https://git.evergreen-ils.org/?p=Evergreen.git;a=commitdiff;h=958b49f2d12e27a535e47be55526413f0e0f285d> |
16:25 |
pinesol |
News from commits: LP#2404514: Disable StrictHostKeyChecking in Net::SFTP::Foreign <https://git.evergreen-ils.org/?p=Evergreen.git;a=commitdiff;h=0d3e221f6452ab6f463262909569dfc46d35d9ae> |
16:25 |
pinesol |
News from commits: LP#2040514: Followup repairs <https://git.evergreen-ils.org/?p=Evergreen.git;a=commitdiff;h=fac2c6dbeffd1a7062050924ff4cd5f31fe10900> |
16:25 |
pinesol |
News from commits: LP#2040514: Re-implement Acq EDI SFTP. <https://git.evergreen-ils.org/?p=Evergreen.git;a=commitdiff;h=3c83f014fadac299ca1b7726c077578fcc406dd1> |
16:26 |
mmorgan |
JBoyer++ |
16:26 |
Bmagic |
ty mmorgan |
16:27 |
Dyrcona |
JBoyer++ |
16:27 |
* Dyrcona |
signs out. |
17:03 |
|
mmorgan left #evergreen |
17:45 |
kmlussier |
Dyrcona++ Bmagic++ JBoyer++ csharp__++ |
18:10 |
|
kmlussier left #evergreen |