Merge pull request #60 from Novartis/59_s3_caching

#59 add refresh query param to force refresh of S3 cache

.github/workflows/pr-checks.yaml

@@ -39,6 +39,7 @@ jobs:
           conda env create -f environment.yml
           eval "$(conda shell.bash hook)"
           conda activate cellxgene-gateway
+          pip install markupsafe==2.0.1 # temporary workaround for jinja2-2.11.3 calling soft_unicode in markupsafe
           python setup.py install
 
       - name: Run tests

README.md

@@ -76,6 +76,7 @@ Optional environment variables:
 * `GATEWAY_EXTRA_SCRIPTS` - JSON array of script paths, will be embedded into each page and forwarded with `--scripts` to cellxgene server
 * `GATEWAY_ENABLE_ANNOTATIONS` - Set to `true` or to `1` to enable cellxgene annotations. 
 * `GATEWAY_ENABLE_BACKED_MODE` - Set to `true` or to `1` to load AnnData in file-backed mode. This saves memory and speeds up launch time but may reduce overall performance.
+* `S3_ENABLE_LISTINGS_CACHE` - Set to `true` or to `1` to cache listings of S3 folders for performance. Can be overridden by setting `filecrawl.html?refresh=true` query parameter.
 
 If any of the following optional variables are set, [ProxyFix](https://werkzeug.palletsprojects.com/en/1.0.x/middleware/proxy_fix/) will be used.
 * `PROXY_FIX_FOR` - Number of upstream proxies setting X-Forwarded-For

cellxgene_gateway/gateway.py

@@ -22,9 +22,7 @@ from flask import (
     send_from_directory,
     url_for,
 )
-from flask_api import status
 from werkzeug.middleware.proxy_fix import ProxyFix
-from werkzeug.utils import secure_filename
 
 from cellxgene_gateway import env, flask_util
 from cellxgene_gateway.backend_cache import BackendCache

cellxgene_gateway/items/s3/s3item_source.py

@@ -7,9 +7,11 @@
 # OR CONDITIONS OF ANY KIND, either express or implied. See the License for
 # the specific language governing permissions and limitations under the License.
 
-from os.path import basename, dirname, join
+import os
+from os.path import basename, dirname
 from typing import List
 
+import flask
 import s3fs
 
 from cellxgene_gateway import dir_util
@@ -18,6 +20,10 @@ from cellxgene_gateway.items.item_source import ItemSource, LookupResult
 from cellxgene_gateway.items.s3.s3item import S3Item
 
 
+def truthy(val: str):
+    return val.lower() in ["true", "1"]
+
+
 class S3ItemSource(ItemSource):
     def __init__(
         self,
@@ -28,10 +34,9 @@ class S3ItemSource(ItemSource):
         annotation_file_suffix=".csv",
     ):
         self._name = name
-        disable_cache = os.environ.get("S3_DISABLE_LISTINGS_CACHE", "false").lower()
-        assert disable_cache in ['0', '1', 'false', 'true']
-        self.use_listings_cache = disable_cache.lower() not in ["0", "false"]
-
+        enable_cache = os.environ.get("S3_ENABLE_LISTINGS_CACHE", "false").lower()
+        assert enable_cache in ["0", "1", "false", "true"]
+        self.use_listings_cache = truthy(enable_cache)
         self.s3 = s3fs.S3FileSystem(use_listings_cache=self.use_listings_cache)
         if bucket.startswith("s3://"):
             raise Exception(
@@ -71,6 +76,13 @@ class S3ItemSource(ItemSource):
         item_tree = self.scan_directory("" if filter is None else filter)
         return item_tree
 
+    @property
+    def refresh(self):
+        return (
+            truthy(flask.request.args.get("refresh", default="false"))
+            or not self.use_listings_cache
+        )
+
     def scan_directory(self, directory_key="") -> dict:
         url = self.url(directory_key)
 
@@ -79,7 +91,7 @@ class S3ItemSource(ItemSource):
 
         s3key_map = dict(
             (self.remove_bucket(filepath), "s3://" + filepath)
-            for filepath in sorted(self.s3.ls(url, refresh=not self.use_listings_cache))
+            for filepath in sorted(self.s3.ls(url, refresh=self.refresh))
         )
 
         def is_annotation_dir(dir_s3key):
@@ -170,7 +182,9 @@ class S3ItemSource(ItemSource):
                 self.make_s3item_from_key(
                     basename(annotation), self.remove_bucket(annotation), True
                 )
-                for annotation in sorted(self.s3.ls(annotations_fullpath, refresh=not self.use_listings_cache))
+                for annotation in sorted(
+                    self.s3.ls(annotations_fullpath, refresh=self.refresh)
+                )
                 if annotation.endswith(self.annotation_file_suffix)
                 and self.s3.isfile("s3://" + annotation)
             ]

tests/items/s3/test_s3item_source.py

@@ -24,7 +24,10 @@ class TestScanDirectory(unittest.TestCase):
         )
 
     @patch("s3fs.S3FileSystem")
-    def test__GIVEN_multilevel_bucket_THEN_properly_recurses_suburls(self, s3func):
+    @patch("flask.request")
+    def test__GIVEN_multilevel_bucket_THEN_properly_recurses_suburls(
+        self, requestMock, s3func
+    ):
         class S3Mock:
             def exists(path):
                 if path in [
@@ -38,7 +41,8 @@ class TestScanDirectory(unittest.TestCase):
                     return True
                 raise Exception("exists called with " + path)
 
-            def ls(path):
+            def ls(path, refresh):
+                assert refresh == True
                 if path == "s3://my-bucket/":
                     return [
                         "my-bucket/lvl1",
@@ -79,6 +83,7 @@ class TestScanDirectory(unittest.TestCase):
                 raise Exception("isfile called with " + path)
 
         s3func.return_value = S3Mock
+        requestMock.args.get.return_value = "true"
         source = S3ItemSource("my-bucket")
         tree = source.scan_directory()