spacepy / dbprocessing

Automated processing controller for heliophysics data
5 stars 4 forks source link

add postgresql support and unit testing #78

Closed dnadeau-lanl closed 3 years ago

dnadeau-lanl commented 3 years ago

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.

add posgresql dialect and fix unit testings. Some failure in testing are due to the other of files.

jtniehof commented 3 years ago

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.

jtniehof commented 3 years ago

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.

dnadeau-lanl commented 3 years ago

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! 😏

jtniehof commented 3 years ago

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.

jtniehof commented 3 years ago

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.

dnadeau-lanl commented 3 years ago

This looks amazing! 🎉 🎆 I think we are ready to merge this. I will start using it here.

Thanks for all that work!