# Port ctrl_provenance to Python 3 and update unit tests for pytest

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:

#### Activity

Hide
Russell Owen added a comment - - edited

Jenkins py2 passes. Need the cat package to be updated before testing py3

Show
Russell Owen added a comment - - edited Jenkins py2 passes. Need the cat package to be updated before testing py3
Hide
Tim Jenness added a comment -

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.

Show
Tim Jenness added a comment - 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.
Hide
Russell Owen added a comment -

There was no real memory leak, just missing content in the tearDown method. All is well (at least in Python 2)

Show
Russell Owen added a comment - There was no real memory leak, just missing content in the tearDown method. All is well (at least in Python 2)
Hide
Tim Jenness added a comment -

Builds for me (now that cat has been merged).

Show
Tim Jenness added a comment - Builds for me (now that cat has been merged).
Hide
Russell Owen added a comment -

Steve, do you have time to look at this?

Show
Russell Owen added a comment - Steve, do you have time to look at this?
Hide
Steve Pietrowicz added a comment -

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.

Show
Steve Pietrowicz added a comment - 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.
Hide
Tim Jenness added a comment -

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?

Show
Tim Jenness added a comment - 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?
Hide
Steve Pietrowicz added a comment -

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. Show Steve Pietrowicz added a comment - 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.
Hide
Tim Jenness added a comment -

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.

Show
Tim Jenness added a comment - 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.
Hide
Tim Jenness added a comment -

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.

Show
Tim Jenness added a comment - 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 .
Hide
Tim Jenness added a comment -

Ideally we'd deprecate next and rename it to nextResult in daf_persistence.

Show
Tim Jenness added a comment - Ideally we'd deprecate next and rename it to nextResult in daf_persistence .
Hide
Russell Owen added a comment -

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.

Show
Russell Owen added a comment - 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.
Hide
Russell Owen added a comment -

I also filed DM-7323 as per Tim Jenness suggestion about renaming DbStorage.next

Show
Russell Owen added a comment - I also filed DM-7323 as per Tim Jenness suggestion about renaming DbStorage.next
Hide
Russell Owen added a comment - - edited

Jenkins Python 2 passed and Steve Pietrowicz enhanced the documentation for ProvenanceSetup.addWorkflowRecordCmd. Merged to master.

Show
Russell Owen added a comment - - edited Jenkins Python 2 passed and Steve Pietrowicz enhanced the documentation for ProvenanceSetup.addWorkflowRecordCmd . Merged to master.

#### People

Assignee:
Russell Owen
Reporter:
Russell Owen
Reviewers:
Steve Pietrowicz
Watchers:
Andy Salnikov, Nate Pease [X] (Inactive), Russell Owen, Steve Pietrowicz, Tim Jenness