Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-9918

Robustify sqlite use

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: daf_persistence
    • Labels:
      None
    • Team:
      Data Access and Database

      Description

      Sogo Mineo hit some test failures because of a broken sqlite3 module. He writes:

      At the beginning of
      daf_persistence/13.0-2-g7f7a1db+1/python/lsst/daf/persistence/registries.py ,
      "haveSqlite3" was set to False because "import sqlite3" failed.

      At line 77, the IF block was skipped because "haveSqlite3" is False.
      The execution continued to reach line 89,
      and PosixRegistry(root="filename.sqlite3") was created.

      I think one of the following modifications is desirable:

      1. Raise error when "not haveSqlite3 and re.match(r'.*\.sqlite3', location)"
      before line 77.

      2. Alter os.path.exists(location) to os.path.isdir(location) at line 88.

      Line numbers refer to the HSC master branch, at https://github.com/HyperSuprime-Cam/daf_persistence

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -
            Show
            price Paul Price added a comment - Passed Jenkins .
            Hide
            price Paul Price added a comment -

            Nate Pease, would you please review these small changes?

            price@pap-laptop:~/LSST/daf_persistence (tickets/DM-9918=) $ git sub
            commit 9d1d7a93892c959ac49838fa531279c0ee562cc0
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Mar 29 20:33:03 2017 -0400
             
                Registry: fail if given a sqlite3 registry but import failed
                
                If we're given a sqlite3 registry, then the user expects us
                to use it. But if we failed to import the sqlite3 module then
                there's nothing we can do so we should raise an exception.
             
             python/lsst/daf/persistence/registries.py | 5 ++++-
             1 file changed, 4 insertions(+), 1 deletion(-)
             
            commit c20d59ad35c184ae13a2ad2089dc9ced12eec7db
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Mar 29 20:35:03 2017 -0400
             
                Registry: a PosixRegistry should be a directory
                
                It doesn't make sense for the PosixRegistry to be a file.
             
             python/lsst/daf/persistence/registries.py | 2 +-
             1 file changed, 1 insertion(+), 1 deletion(-)
            

            Show
            price Paul Price added a comment - Nate Pease , would you please review these small changes? price@pap-laptop:~/LSST/daf_persistence (tickets/DM-9918=) $ git sub commit 9d1d7a93892c959ac49838fa531279c0ee562cc0 Author: Paul Price <price@astro.princeton.edu> Date: Wed Mar 29 20:33:03 2017 -0400   Registry: fail if given a sqlite3 registry but import failed If we're given a sqlite3 registry, then the user expects us to use it. But if we failed to import the sqlite3 module then there's nothing we can do so we should raise an exception.   python/lsst/daf/persistence/registries.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)   commit c20d59ad35c184ae13a2ad2089dc9ced12eec7db Author: Paul Price <price@astro.princeton.edu> Date: Wed Mar 29 20:35:03 2017 -0400   Registry: a PosixRegistry should be a directory It doesn't make sense for the PosixRegistry to be a file.   python/lsst/daf/persistence/registries.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
            Hide
            price Paul Price added a comment -

            Ping: Nate Pease, are you able to review this, please?

            Show
            price Paul Price added a comment - Ping: Nate Pease , are you able to review this, please?
            Hide
            npease Nate Pease added a comment -

            the changes look fine. There might be an issue with line length, and use of % instead of .format. I'm fine with whatever you decide to do about those.

            Show
            npease Nate Pease added a comment - the changes look fine. There might be an issue with line length, and use of % instead of .format. I'm fine with whatever you decide to do about those.
            Hide
            price Paul Price added a comment -

            Thanks Nate Pease. On both points I followed the current style in that file.

            Merged to master.

            Show
            price Paul Price added a comment - Thanks Nate Pease . On both points I followed the current style in that file. Merged to master.

              People

              • Assignee:
                price Paul Price
                Reporter:
                price Paul Price
                Reviewers:
                Nate Pease
                Watchers:
                Nate Pease, Paul Price
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: