Merge pull request #253 from 4dn-dcic/kmp_fewer_warnings_20230410

Fewer snovault pytest and sqlalchemy warnings

CHANGELOG.rst

@@ -6,6 +6,21 @@ snovault
 Change Log
 ----------
 
+
+8.0.1
+=====
+
+* Fix some warnings from ``pytest``
+
+  * If a method has "test" in its name but isn't a test, it needs a prefix "_"
+
+* Fix some warnings from ``sqlalchemy``
+
+  * ``session.connection()`` doesn't need to ``.connect()``
+  * ``.join(x, y, ...)`` should be ``.join(x).join(y)...``
+  * ``session.query(Foo).get(bar)`` should be ``session.get(Foo, bar)``
+
+
 8.0.0
 =====
 

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "dcicsnovault"
-version = "8.0.0"
+version = "8.0.1"
 description = "Storage support for 4DN Data Portals."
 authors = ["4DN-DCIC Team <[email protected]>"]
 license = "MIT"

snovault/storage.py

@@ -375,7 +375,9 @@ def get_by_json(self, key, value, item_type, default=None):
         try:
             # baked query seem to not work with json
             query = (session.query(CurrentPropertySheet)
-                     .join(CurrentPropertySheet.propsheet, CurrentPropertySheet.resource)
+                     # Rewrittent to use two separate joins per SQLAlchemy 2.0 requirements. -kmp 10-Apr-2023
+                     .join(CurrentPropertySheet.propsheet)
+                     .join(CurrentPropertySheet.resource)
                      .filter(Resource.item_type == item_type,
                              PropertySheet.properties[key].astext == value)
                      )
@@ -480,7 +482,8 @@ def update(self, model, properties=None, sheets=None, unique_keys=None, links=No
             msg = 'Cannot update because of one or more conflicting (or undefined) UUIDs'
             raise HTTPConflict(msg)
         assert unique_keys is not None
-        conflicts = [pk for pk in keys_add if session.query(Key).get(pk) is not None]
+        # Formerly session.query(Key).get(pk), rewritten for SA2.0
+        conflicts = [pk for pk in keys_add if session.get(Key, pk) is not None]
         assert conflicts
         msg = 'Keys conflict: %r' % conflicts
         raise HTTPConflict(msg)
@@ -524,7 +527,7 @@ def _update_keys(self, model, unique_keys):
 
         session = self.DBSession()
         for pk in to_remove:
-            key = session.query(Key).get(pk)
+            key = session.get(Key, pk)  # formerly session.query(Key).get(pk), rewritten for SA2.0
             session.delete(key)
 
         for name, value in to_add:
@@ -548,7 +551,8 @@ def _update_rels(self, model, links):
         to_add = rels - existing
 
         for rel, target in to_remove:
-            link = session.query(Link).get((source, rel, target))
+            # formerly link = session.query(Link).get((source, rel, target)), rewritten for SA2.0
+            link = session.get(Link, (source, rel, target))
             session.delete(link)
 
         for rel, target in to_add:
@@ -640,7 +644,7 @@ def get_blob(self, download_meta):
         if isinstance(blob_id, str):
             blob_id = uuid.UUID(blob_id)
         session = self.DBSession()
-        blob = session.query(Blob).get(blob_id)
+        blob = session.get(Blob, blob_id)  # was session.query(Blob).get(blob_id), rewritten for SA2.0
         return blob.data
 
 

snovault/tests/test_attachment.py

@@ -251,7 +251,7 @@ class TestAttachmentEncrypted:
     """
     url = '/testing-downloads/'
 
-    def testing_encrypted_download(self, testapp):
+    def _testing_encrypted_download(self, testapp):  # not a test, just does some setup
         item = {
             'attachment': {
                 'download': 'red-dot.png',
@@ -270,10 +270,10 @@ def create_bucket_and_post_download(self, testapp):
         blob_bucket = 'encoded-4dn-blobs'  # note that this bucket exists but is mocked out here
         conn = boto3.resource('s3', region_name='us-east-1')
         conn.create_bucket(Bucket=blob_bucket)
-        return self.testing_encrypted_download(testapp)
+        return self._testing_encrypted_download(testapp)
 
     @staticmethod
-    def get_attachment_from_s3(client, url):
+    def _get_attachment_from_s3(client, url):
         """ Uses ff_utils.parse_s3_bucket_and_key_url to parse the s3 URL in our data into it's
             bucket, key pairs and acquires/reads the content.
         """
@@ -282,12 +282,12 @@ def get_attachment_from_s3(client, url):
 
     def attachment_is_red_dot(self, client, url):
         """ Fails assertion if the url is not the RED_DOT """
-        content = self.get_attachment_from_s3(client, url)
+        content = self._get_attachment_from_s3(client, url)
         assert content == b64decode(RED_DOT.split(',', 1)[1])
 
     def attachment_is_blue_dot(self, client, url):
         """ Fails assertion if the url is not the BLUE_DOT """
-        content = self.get_attachment_from_s3(client, url)
+        content = self._get_attachment_from_s3(client, url)
         assert content == b64decode(BLUE_DOT.split(',', 1)[1])
 
     @mock_s3

snovault/tests/test_indexing.py

@@ -128,7 +128,7 @@ def setup_and_teardown(app):
 
     # AFTER THE TEST
     session = app.registry[DBSESSION]
-    connection = session.connection().connect()
+    connection = session.connection()  # was session.connection().connect(), rewritten for SA2.0
     # The reflect=True argument to MetaData was deprecated. Instead, one is supposed to call the .reflect()
     # method after creation. (This comment is transitional and can go away if things seem to work normally.)
     # -kmp 11-May-2020
@@ -1960,7 +1960,8 @@ def test_assert_transactions_table_is_gone(app):
     """
     session = app.registry[DBSESSION]
     conn = session.connection()
-    conn.connect()
+    # In SQLAlchemy 2.0, it is no longer necessary to explicitly connect.
+    # conn.connect()
     # The reflect=True argument to MetaData was deprecated. Instead, one is supposed to call the .reflect()
     # method after creation. (This comment is transitional and can go away if things seem to work normally.)
     # -kmp 11-May-2020

snovault/tests/test_storage.py

@@ -115,10 +115,11 @@ def test_get_by_json(session):
     resource = session.query(Resource).one()
     session.flush()
     query = (session.query(CurrentPropertySheet)
-             .join(CurrentPropertySheet.propsheet, CurrentPropertySheet.resource)
+             # Rewrittent to use two separate joins per SQLAlchemy 2.0 requirements. -kmp 10-Apr-2023
+             .join(CurrentPropertySheet.propsheet)
+             .join(CurrentPropertySheet.resource)
              .filter(PropertySheet.properties['foo'].astext == 'baz')
              )
-
     data = query.one()
     assert data.propsheet.properties == props2