Fix Version/s: None
We may want to consider adding the tests but skipping them so that we get a reminder that they are disabled. We can do that by adding a setUp function to the test class and have it immediately raise a skip exception.
There was no real memory leak, just missing content in the tearDown method. All is well (at least in Python 2)
This all looks fine. The only thing I noticed was that the tests inherited from unittest.TestCase instead of lsst.utils.tests.TestCase. I'm not sure that it completely required, but that's what they did in the SQR-12.lsst.io porting guide.
You only need to inherit from utils TestCase if you are using asserts from that test case. So it doesn't really matter.
I did have a quick look at the PR and I have a concern over the iterator in daf_persistence. I can't actually test the DbStorage_1.py usage of next because the tests skip for me even if I'm on lsst-dev. I need to know how the end of the iterator is triggered from C++. It might be that I have to intercept some other condition and translate it to StopIteration. Steve Pietrowicz do you know what machine I have to be on for the DbStorage_1.py test to run or is the server not actually active any more?
I believe what's happening in DbStorage_1.py is that it checks the $HOME/.lsst/db-auth.paf file for auth info for lsst10.ncsa.uiuc.edu (this really should be lsst10.ncsa.illinois.edu) at port 3306. If you do not have that entry in that file, the test will be skipped.
Thank you. That explains why I had a memory of the tests running okay on my laptop and I was confused on my iMac and lsst-dev. I'll take a quick look at daf_persistence. Russell Owen can we hold off merging until I've worked out whether we need to be catching StopIteration in the code treating it as a proper Python 3 iterator.
Ok. It looks like daf_persistence::next is not actually an iterator and should not be confused as one. It's job is just to move a pointer to the next result and not to return the result so it should never be used like a Python iterator. The name is a major source of confusion when switching from C++ to Python3. It looks like I should revert
DM-7226 and ctrl_provenance should go back to using the next() method and not assuming it's at all related to the Python next.
Ideally we'd deprecate next and rename it to nextResult in daf_persistence.
As per Tim Jenness request I changed next(self._globalDb) to self._globalDb.next() in dc3.py. I also fixed flake8 warnings and errors and standardized imports. I'm ready to merge after a Jenkins run.
I also filed
DM-7323 as per Tim Jenness suggestion about renaming DbStorage.next
Jenkins Python 2 passed and Steve Pietrowicz enhanced the documentation for ProvenanceSetup.addWorkflowRecordCmd. Merged to master.
Jenkins py2 passes. Need the cat package to be updated before testing py3