add postgresql support and unit testing #78
Conversation
|
I've rebased around the merges and force-pushed to this PR. I kept the original branch at https://github.com/jtniehof/dbprocessing/tree/postgresql-original for reference. I checked the diffs of every commit in the new branch and compared to the original and I think I'm capturing it properly. I'm going to go ahead and comment on this, and if we agree on changes I'll probably rebase them in to keep the history reasonably clear. |
* Not tested in the sense that DButils isn't unit tested for postgres, but test added that will catch this when all unit tests run against postgres * Simply trying and catching doesn't work, because leaves the DB in a state that needs to be rolled back
There was a problem hiding this comment.
Massively rebased and all the checks still pass (phew.) I have two open questions in the specific comments. Once I have feedback on those I can do one more rebase and we can merge.
Please do look over the current diff though and see if it all makes sense to you--my fingerprints are all over this now so I'd appreciate having another set of eyes, particularly making sure it's still fit for your purpose.
|
I should warn you that, at this point, if you run the unit tests and PGDATABASE is set, it WILL wipe out your database in the teardown! I'm thinking it might be worth adding a check to the setUp that the database is really empty (probably just a quick check that there's no mission defined yet.) That way if you forget and run the unit tests on a machine with a connection to an operational db you won't be hosed. |
|
That's a good idea. Right now I am sourcing a file with "UNH" to set the testing table. I am always running dbprocessing "unit_test" in a "devel" machine, so that machine does not have access to the "real" database. But I would like the safety, since things happen! 😏 |
|
I'll do another rebase tomorrow morning and include the safety checking...then should be good to merge if it all looks good to you. |
* Read in segments to avoid using all of memory.
* Patches things up if PGDATABASE is mis-set.
|
Okay, rebase done, including the safety check. Take a look and post if you're happy, and then I'll do the approve and merge. |
|
This looks amazing! 🎉 🎆 Thanks for all that work! |
PR Checklist
New code is easier to review, integrate and maintain if it's
consistent with the style of the rest of the dbprocessing code.
Please try to follow as much of this checklist as you can for
your PR. If a given item is not relevant to your PR, please check
it off, add "(N/A)" to the start of the line, and include a
description below the checklist.
If you don't know how to complete something in the checklist, go
ahead and submit the PR as a draft and ask for help.
Some additional suggestions are given below.
Please also see our Code of Conduct so that the dbprocessing community
remains welcoming and inclusive.
Go ahead and replace the text above (and this line!) with your
pull request description.
Closes #if this PR closes the issue, or some other reference, such asSee #if it is related in some other way)add posgresql dialect and fix unit testings. Some failure in testing are due to the other of files.