csingley / ofxtools

Python OFX Library
Other
301 stars 68 forks source link

Add User Configuration File CLI Argument #147

Open patbakdev opened 2 years ago

patbakdev commented 2 years ago

I have a use case that requires two sets of books (personal and business) that I like to keep separate. In order to do this I wanted to be able to pass in the config file instead of using the one in my HOME directory. I added a CLI override (--config) to ofxget that allows me to do that.

I tried to match the existing coding style as much as possible.

I also added a very simple test and ran make test. Everything passed expect for mypy ofxtool which failed with 6 errors that I believe where there before my changes.

csingley commented 2 years ago

Everything passed expect for mypy ofxtool which failed with 6 errors that I believe where there before my changes.

Indeed, these just crept in last week & I haven't had time to fix 'em yet.

I am not necessarily opposed to this change, and I appreciate your effort here - but in the name of simplicity, I do have to ask: are you sure you can't support your use case with a single config file? I have done quite a bit of what you're talking about, using a config file that contains entries like [chase-personal-checking], [chase-personal-cc], [chase-business-checking], [chase-business-cc], etc.

You know, there's nothing magical about the names in fi.cfg or ofxget.cfg. You can just copy working config info out of fi.cfg (try using e.g. ofxget --list chase) under a new section named whatever you please, adding in acct info, userid, etc. The custom config section will be available to ofxget at the CLI.

For more throughgoing automation & customization, I'd suggest just ditching the ofxget front end altogether. Just write your own calls to ofxtools.Client.OFXClient, including whatever configs you want in your own Python script. It's really pretty simple to do, and will make for a much cleaner cron job (or what have you) in your automation workflow.

There's nothing obnoxious about modifying ofxget to accept a --config arg, but what advantages does it bring?

patbakdev commented 2 years ago

I am not necessarily opposed to this change, and I appreciate your effort here - but in the name of simplicity, I do have to ask: are you sure you can't support your use case with a single config file?

Do-able? Yes. But for legal reasons I prefer to keep things separate.

You know, there's nothing magical about the names in fi.cfg or ofxget.cfg. You can just copy working config info out of fi.cfg

This just seems overly complicated to me and something everyone with this requirement would have to re-invent.

For more throughgoing automation & customization, I'd suggest just ditching the ofxget front end altogether. Just write your own calls to ofxtools.Client.OFXClient, including whatever configs you want in your own ython script.

This is certainly an option. I have debated going this route in the past and could easily implement it I think. In fact, I am slightly leaning toward it given your feedback. That being said, I still find ofxget a great CLI tool.

I suppose I could update my scripts to use custom configuration data from my ledger files to call into ofxtools and just use the full set of command line options for any one-off cases. One less config file would be nice.

There's nothing obnoxious about modifying ofxget to accept a --config arg, but what advantages does it bring?

I was using a another CLI tool called OFXClient and my script would basically call it via a subprocess call. It was great in that it would create a single ofx file with all of my accounts and had a similarly simplistic config file, but I haven't seen any activity on it for a while and when I ran into the recent Fidelity 403 issue I thought I would try and switch it out for ofxget since this project seems to be more active. One advantage I like about the CLI approach is that I can do quick one-off downloads or check settings, etc, without having to change my scripts.

Its not a very complicated change (code amount wise) so we could leave the pull request open for a bit to see if anyone else thinks it would be useful, but if you don't think this change makes sense for the project I completely understand, Either way hacking on the code was fun and thanks for considering the change.

csingley commented 2 years ago

This just seems overly complicated to me and something everyone with this requirement would have to re-invent.

It could hardly be simpler. Just run ofxget list <your FI's nickname>, then copy/paste the listing into ofxget.cfg, under a different heading. Edit as you see fit; it's just standard INI syntax, and I've provided documentation.

Give it a shot and see if you find it all that intimidating. It's certainly easier than writing tests!

I'm not aware of any legal woes that arise from mixing your peanut butter with your chocolate in this manner... i.e. commingling metadata about personal/business funds, rather than commingling the funds themselves. However I'm always open to increasing my awareness. Am I whistling past the graveyard here?

In any case, I don't demand that you justify your preference for multiple config files. Just wondering if you've thoroughly evaluated the alternatives.

Its not a very complicated change (code amount wise) so we could leave the pull request open for a bit to see if anyone else thinks it would be useful

Oh yeah no doubt. I don't feel dogmatic about it, it just sounds a little odd to me. But thenI don't actually use ofxget much myself; the script is fairly icky & heavyweight. I'm happy that you find ofxget useful, and if people want this it's fine by me.

Is there a second for this proposal?

patbakdev commented 2 years ago

I have switched over to using the library directly as you suggested and that is now solving my requirements. I still think there is value in having the switch, but I can go either way now that I got it working.

csingley commented 2 years ago

I don't mind adding the switch if demand for it overcome mild backpressure against the added LOC.

But I really do think that, if you can code simple Python, that's really a better way to go. Glad you got it working to your satisfaction.