Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: suport btrs (bai v3) files #118

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

omerlh
Copy link
Contributor

@omerlh omerlh commented May 15, 2024

fix #117

@adamdecaf
Copy link
Member

Can we add a test verifying the fields of the version 3 file. Are there really no differences between v2 and v3?

@omerlh
Copy link
Contributor Author

omerlh commented May 16, 2024

From a quick look at the BTRS file I have I think it's the same, it's also mentioned in the spec that in general the file format is the same, there are some difference in BTRS files requires changes to the parser. Maybe this can be merged with a warning of "btrs is experimental"?

@adamdecaf
Copy link
Member

I'm trying to decide if we need a new package with the differences, or if we can just change behavior based off the version number.

Screenshot 2024-05-16 at 09 17 12

@adamdecaf
Copy link
Member

I've started on a BTRS test to verify we're parsing as expected. I'm using their examples and want to get all of that copied over in a test before we merge.

Can you help out @omerlh ? I'll get chunks of the test added over today/tomorrow.

@omerlh
Copy link
Contributor Author

omerlh commented May 16, 2024

Yeah sure I am taking a look now, let me see what I can do in the next hour or so

@omerlh
Copy link
Contributor Author

omerlh commented May 16, 2024

Well I was mistaken, the files I have has version 3 in the header:

01,redacted,redacted,redacted,redacted,redacted,,,3/

But it's actually BAI

Take a look at the next line, in BAI2 it's group header and in BTRS it called "bank header". This BTRS line cannot be parsed correctly:

02,,122099999,1,150622,,,2/ 

This is from the official example...

So.. what I need is BAI2 parser that can eat fake BTRS files 😂 Do you want this repository to support BTRS parsing? Or have BTRS parser in a different repository?

@adamdecaf
Copy link
Member

I'm thinking a moov-io/btrs repository would be better then. I could get behind a move towards pkg/bai2 and pkg/btrs subfolders of this repository too.

@omerlh
Copy link
Contributor Author

omerlh commented May 16, 2024

Yeah that makes sense, just need something for weird banks that sends BTRS3 files that are actually bai2 🤷

@adamdecaf
Copy link
Member

I don't understand how you're getting BTRS3 files that are actually bai2. Are the files a mix of bai2 and BTRS records?

@omerlh
Copy link
Contributor Author

omerlh commented May 19, 2024

I think it's just wrong version, so for my use case a flag like ignoreVersion will be better, WDYT?

@adamdecaf
Copy link
Member

Does the current bai2 parser work with an invalid version? If not we can add an override to skip invalid version headers.

BTR3 looks to need a new parser/project/package.

@omerlh
Copy link
Contributor Author

omerlh commented May 26, 2024

Yes I agree about BTRS3 and the flag, if that's ok with you I can open a PR that will solve my issue :)

@adamdecaf
Copy link
Member

Yea go ahead and open a PR for your issue. I'm back and will look over this PR.

@adamdecaf
Copy link
Member

can we refresh this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support BTRS3 (bai3)
2 participants