#81 added unit test for gene sets

This adds the flag `GATEWAY_ENABLE_GENE_SETS` to enable support for gene
sets.  To simplify implementation, activating this flag also activates
`GATEWAY_ENABLE_ANNOTATIONS`.  The gene sets are saved in a file that
has the same name as the annotations `csv` but with `_gene_sets`
appended to the file name (before the extension).  This file is hidden
in filecrawler, and the gene sets are loaded when the associated
annotations file is loaded.

If the annotations file is missing, then an Exception is raised.

I have updated one unit test to make it expect
`--disable-gene-sets-save` in the default case (i.e. if
`GATEWAY_ENABLE_ANNOTATIONS = 0`).  All units tests pass.

I have updated the README to document `GATEWAY_ENABLE_GENE_SETS`.

.github/workflows/pr-checks.yaml

@@ -6,7 +6,7 @@ on: [push, pull_request]
 
 jobs:
   black:
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
         name: Checkout repository
@@ -25,7 +25,7 @@ jobs:
           black . --check
   # This job is copied over from `deploy.yaml`
   run-tests:
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
 

Changelog.md

@@ -1,3 +1,18 @@
+# 0.3.11
+
+* #81 added support for gene sets
+
+# 0.3.10
+
+* #65 Added GATEWAY_EXPIRE_SECONDS to set how long cellxgene servers can remain idle before being terminated.
+* Added GATEWAY_LOG_LEVEL to set the log level
+* #68 Close connections after reading response
+* #68 Background thread reads from output of cellxgene process until it exits
+
+# 0.3.9
+
+* Added S3_ENABLE_LISTINGS_CACHE variable (See README.md)
+
 # 0.3.8
 
 * Fixed bug #57 affecting deeply nested subdirectory listing

Dockerfile

@@ -0,0 +1,8 @@
+FROM python:3.9
+
+RUN pip install cellxgene-gateway 'MarkupSafe<2.1'
+
+ENV CELLXGENE_DATA=/cellxgene-data
+ENV CELLXGENE_LOCATION=/usr/local/bin/cellxgene
+
+CMD ["cellxgene-gateway"]

README.md

@@ -73,10 +73,12 @@ Optional environment variables:
 * `EXTERNAL_PROTOCOL` - typically http when running locally, can be https when deployed if the gateway is behind a load balancer or reverse proxy that performs https termination. Default value "http"
 * `GATEWAY_IP` - ip addess of instance gateway is running on, mostly used to display SSH instructions. Defaults to `socket.gethostbyname(socket.gethostname())`
 * `GATEWAY_PORT` - local port that the gateway should bind to, defaults to 5005
+* `GATEWAY_EXPIRE_SECONDS` - time in seconds that a cellxgene process will remain idle before being terminated. Defaults to 3600 (one hour)
 * `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_ANNOTATIONS` - Set to `true` or to `1` to enable cellxgene annotations and gene sets.
 * `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.
+* `GATEWAY_LOG_LEVEL` - default is `INFO`. set to `DEBUG` to increase logging and to `WARNING` to decrease logging.
+* `S3_ENABLE_LISTINGS_CACHE` - Set to `true` or to `1` to cache listings of S3 folders for performance. If the cache becomes stale, set `filecrawl.html?refresh=true` query parameter to refresh the cache.
 
 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
@@ -87,6 +89,33 @@ If any of the following optional variables are set, [ProxyFix](https://werkzeug.
 
 The defaults should be fine if you set up a venv and cellxgene_data folder as above.
 
+## Running cellxgene-gateway with Docker
+
+First, build Docker image:
+
+```bash
+docker build -t cellxgene-gateway .
+```
+
+Then, cellxgene-gateway can be launched as such:
+
+```bash
+docker run -it --rm \
+-v <local_data_dir>:/cellxgene-data \
+-p 5005:5005 \
+cellxgene-gateway
+```
+
+Additional environment variables can be provided with the `-e` parameter:
+
+```bash
+docker run -it --rm \
+-v <local_data_dir>:/cellxgene-data \
+-e GATEWAY_PORT=8080 \
+-p 8080:8080 \
+cellxgene-gateway
+```
+
 # Customization
 
 The current paradigm for customization is to modify files during a build or deployment phase:

cellxgene_gateway/__init__.py

@@ -7,4 +7,4 @@
 # OR CONDITIONS OF ANY KIND, either express or implied. See the License for
 # the specific language governing permissions and limitations under the License.
 
-__version__ = "0.3.8"
+__version__ = "0.3.10"

cellxgene_gateway/cache_entry.py

@@ -9,7 +9,6 @@
 import datetime
 import logging
 import re
-import urllib.parse
 from enum import Enum
 
 import psutil
@@ -22,6 +21,8 @@ from cellxgene_gateway.cellxgene_exception import CellxgeneException
 from cellxgene_gateway.flask_util import querystring
 from cellxgene_gateway.util import current_time_stamp
 
+logger = logging.getLogger(__name__)
+
 
 class CacheEntryStatus(Enum):
     loaded = "loaded"
@@ -57,7 +58,6 @@ class CacheEntry:
 
     @classmethod
     def for_key(cls, key, port):
-
         return cls(
             None,
             key,
@@ -111,7 +111,7 @@ class CacheEntry:
             except psutil.NoSuchProcess:
                 pass
 
-            logging.getLogger("cellxgene_gateway").info(f"terminated {terminated}")
+            logger.info(f"terminated {terminated}")
         self.status = CacheEntryStatus.terminated
 
     def rewrite_text_content(self, cellxgene_content):
@@ -169,38 +169,43 @@ class CacheEntry:
 
         full_path = self.cellxgene_basepath() + subpath + querystring()
 
-        if request.method in ["GET", "HEAD", "OPTIONS"]:
-            cellxgene_response = get(full_path, headers=headers)
-        elif request.method == "PUT":
-            cellxgene_response = put(
-                full_path,
-                headers=headers,
-                data=request.data,
-            )
-        elif request.method == "POST":
-            cellxgene_response = post(
-                full_path,
-                headers=headers,
-                data=request.data,
-            )
-        else:
-            raise CellxgeneException(f"Unexpected method {request.method}", 400)
-        content_type = cellxgene_response.headers["content-type"]
-        if "text" in content_type:
-            gateway_content = self.rewrite_text_content(
-                cellxgene_response.content.decode()
-            )
-        else:
-            gateway_content = cellxgene_response.content
+        try:
+            cellxgene_response = None
+            if request.method in ["GET", "HEAD", "OPTIONS"]:
+                cellxgene_response = get(full_path, headers=headers)
+            elif request.method == "PUT":
+                cellxgene_response = put(
+                    full_path,
+                    headers=headers,
+                    data=request.data,
+                )
+            elif request.method == "POST":
+                cellxgene_response = post(
+                    full_path,
+                    headers=headers,
+                    data=request.data,
+                )
+            else:
+                raise CellxgeneException(f"Unexpected method {request.method}", 400)
+            content_type = cellxgene_response.headers["content-type"]
+            if "text" in content_type:
+                gateway_content = self.rewrite_text_content(
+                    cellxgene_response.content.decode()
+                )
+            else:
+                gateway_content = cellxgene_response.content
 
-        resp_headers = {}
-        for h in copy_headers:
-            if h in cellxgene_response.headers:
-                resp_headers[h] = cellxgene_response.headers[h]
+            resp_headers = {}
+            for h in copy_headers:
+                if h in cellxgene_response.headers:
+                    resp_headers[h] = cellxgene_response.headers[h]
 
-        gateway_response = make_response(
-            gateway_content,
-            cellxgene_response.status_code,
-            resp_headers,
-        )
+            gateway_response = make_response(
+                gateway_content,
+                cellxgene_response.status_code,
+                resp_headers,
+            )
+        finally:
+            if cellxgene_response is not None:
+                cellxgene_response.close()
         return gateway_response

cellxgene_gateway/env.py

@@ -9,7 +9,6 @@
 
 import logging
 import os
-import socket
 
 cellxgene_location = os.environ.get("CELLXGENE_LOCATION")
 cellxgene_data = os.environ.get("CELLXGENE_DATA", "")
@@ -24,7 +23,9 @@ external_protocol = os.environ.get(
 )
 ip = os.environ.get("GATEWAY_IP")
 extra_scripts = os.environ.get("GATEWAY_EXTRA_SCRIPTS")
-ttl = os.environ.get("GATEWAY_TTL")
+expire_seconds = int(
+    os.environ.get("GATEWAY_EXPIRE_SECONDS", os.environ.get("GATEWAY_TTL", "3600"))
+)
 enable_annotations = os.environ.get("GATEWAY_ENABLE_ANNOTATIONS", "").lower() in [
     "true",
     "1",
@@ -33,6 +34,7 @@ enable_backed_mode = os.environ.get("GATEWAY_ENABLE_BACKED_MODE", "").lower() in
     "true",
     "1",
 ]
+log_level = logging.getLevelName(os.environ.get("GATEWAY_LOG_LEVEL", "INFO"))
 
 env_vars = {
     "CELLXGENE_LOCATION": cellxgene_location,
@@ -50,9 +52,10 @@ optional_env_vars = {
     "GATEWAY_IP": ip,
     "GATEWAY_PORT": gateway_port,
     "GATEWAY_EXTRA_SCRIPTS": extra_scripts,
-    "GATEWAY_TTL": ttl,
+    "GATEWAY_EXPIRE_SECONDS": expire_seconds,
     "GATEWAY_ENABLE_ANNOTATIONS": enable_annotations,
     "GATEWAY_ENABLE_BACKED_MODE": enable_backed_mode,
+    "GATEWAY_LOG_LEVEL": log_level,
     "CELLXGENE_ARGS": cellxgene_args,
     "CELLXGENE_DATA": cellxgene_data,
     "PROXY_FIX_FOR": proxy_fix_for,

cellxgene_gateway/gateway.py

@@ -80,7 +80,6 @@ cache = BackendCache()
 
 @app.errorhandler(CellxgeneException)
 def handle_invalid_usage(error):
-
     message = f"{error.http_status} Error : {error.message}"
 
     return (
@@ -95,7 +94,6 @@ def handle_invalid_usage(error):
 
 @app.errorhandler(ProcessException)
 def handle_invalid_process(error):
-
     message = []
 
     message.append(error.message)
@@ -299,7 +297,7 @@ def launch():
 
 def main():
     logging.basicConfig(
-        level=logging.INFO,
+        level=env.log_level,
         format="%(asctime)s:%(name)s:%(levelname)s:%(message)s",
     )
     cellxgene_data = os.environ.get("CELLXGENE_DATA", None)

cellxgene_gateway/items/file/fileitem_source.py

@@ -35,6 +35,11 @@ class FileItemSource(ItemSource):
     def name(self):
         return self._name or f"Files:{self.base_path}"
 
+    def is_gene_set(self, path: str) -> bool:
+        return ("_gene_sets" in path or "-gene-sets" in path) and path.endswith(
+            self.annotation_file_suffix
+        )
+
     def is_h5ad_file(self, path: str) -> bool:
         return path.endswith(self.h5ad_suffix) and os.path.isfile(path)
 
@@ -180,6 +185,7 @@ class FileItemSource(ItemSource):
                 self.make_fileitem_from_path(annotation, annotations_subpath, True)
                 for annotation in sorted(os.listdir(annotations_fullpath))
                 if annotation.endswith(self.annotation_file_suffix)
+                and not self.is_gene_set(annotation)
                 and os.path.isfile(os.path.join(annotations_fullpath, annotation))
             ]
         else:

cellxgene_gateway/prune_process_cache.py

@@ -18,7 +18,7 @@ logger = logging.getLogger(__name__)
 class PruneProcessCache:
     def __init__(self, cache):
         self.cache = cache
-        self.expire_seconds = 3600 if env.ttl is None else int(env.ttl)
+        self.expire_seconds = env.expire_seconds
 
     def __call__(self):
         while True:

cellxgene_gateway/subprocess_backend.py

@@ -14,9 +14,15 @@ from flask_api import status
 
 from cellxgene_gateway.cache_entry import CacheEntryStatus
 from cellxgene_gateway.dir_util import make_annotations
-from cellxgene_gateway.env import cellxgene_args, enable_annotations, enable_backed_mode
+from cellxgene_gateway.env import (
+    cellxgene_args,
+    enable_annotations,
+    enable_backed_mode,
+)
 from cellxgene_gateway.process_exception import ProcessException
 
+logger = logging.getLogger(__name__)
+
 
 class SubprocessBackend:
     def __init__(self):
@@ -28,8 +34,11 @@ class SubprocessBackend:
                 extra_args = f" --annotations-dir {make_annotations(file_path)}"
             else:
                 extra_args = f" --annotations-file {annotation_file_path}"
+                gene_sets_file_path = annotation_file_path[:-4] + "_gene_sets.csv"
+                extra_args += f" --gene-sets-file {gene_sets_file_path}"
         else:
             extra_args = " --disable-annotations"
+            extra_args += " --disable-gene-sets-save"
         if enable_backed_mode:
             extra_args += " --backed"
         if not cellxgene_args is None:
@@ -55,7 +64,7 @@ class SubprocessBackend:
             scripts,
             cache_entry.key.annotation_file_path,
         )
-        logging.getLogger("cellxgene_gateway").info(f"launching {cmd}")
+        logger.info(f"launching {cmd}")
         process = subprocess.Popen(
             [cmd], stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True
         )
@@ -84,5 +93,6 @@ class SubprocessBackend:
                 cache_entry.append_output(output)
 
         cache_entry.set_loaded(process.pid)
-
-        return
+        for output in process.communicate():
+            logger.debug(f"cellxgene:{output}")
+        logger.info(f"exiting {cmd}")

environment.yml

@@ -2,9 +2,9 @@ name: cellxgene-gateway
 channels:
 - conda-forge
 dependencies:
-- python=3.7
+- python=3.9
 - requests
-- flask<2.0.0,>=1.0.2
+- flask
 - psutil
 - black
 - twine
@@ -13,6 +13,6 @@ dependencies:
 - pip
 - pip:
   - pre_commit
-  - flask-api==2.0
-  - werkzeug==1.0.1
-  - cellxgene>=0.15
+  - flask-api
+  - werkzeug
+  - cellxgene

requirements.txt

@@ -1,6 +1,6 @@
-cellxgene>=0.15
-flask<2.0.0,>=1.0.2
-flask-api==2.0
-werkzeug==1.0.1
+cellxgene
+flask
+flask-api
+werkzeug
 psutil
 requests

tests/items/s3/test_s3item_source.py

@@ -1,6 +1,7 @@
 import unittest
 from unittest.mock import MagicMock, Mock, patch
 
+from cellxgene_gateway.gateway import app
 from cellxgene_gateway.items.item import ItemType
 from cellxgene_gateway.items.s3.s3item import S3Item
 from cellxgene_gateway.items.s3.s3item_source import S3ItemSource
@@ -24,10 +25,7 @@ class TestScanDirectory(unittest.TestCase):
         )
 
     @patch("s3fs.S3FileSystem")
-    @patch("flask.request")
-    def test__GIVEN_multilevel_bucket_THEN_properly_recurses_suburls(
-        self, requestMock, s3func
-    ):
+    def test__GIVEN_multilevel_bucket_THEN_properly_recurses_suburls(self, s3func):
         class S3Mock:
             def exists(path):
                 if path in [
@@ -83,9 +81,9 @@ 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()
+        with app.test_request_context(query_string="refresh=true") as test_context:
+            tree = source.scan_directory()
 
         def s3item_compare(i1, i2, msg=""):
             self.assertEqual(i1.name, i2.name, "name equals")

tests/test_prune_process_cache.py

@@ -15,7 +15,7 @@ key = CacheKey(
 
 class TestPruneProcessCache(unittest.TestCase):
     @patch("cellxgene_gateway.util.current_time_stamp", new=lambda: 0)
-    @patch("cellxgene_gateway.env.ttl", new="10")
+    @patch("cellxgene_gateway.env.expire_seconds", new=10)
     @patch("cellxgene_gateway.cache_entry.CacheEntry")
     @patch("cellxgene_gateway.cache_entry.CacheEntry")
     def test_GIVEN_one_old_one_new_THEN_prune_old(self, old, new):

tests/test_subprocess_backend.py

@@ -1,7 +1,6 @@
 import unittest
 from unittest.mock import MagicMock, patch
 
-from cellxgene_gateway.backend_cache import BackendCache
 from cellxgene_gateway.cache_entry import CacheEntry
 from cellxgene_gateway.cache_key import CacheKey
 from cellxgene_gateway.items.file.fileitem import FileItem
@@ -33,10 +32,46 @@ class TestSubprocessBackend(unittest.TestCase):
             backend.launch(cellxgene_loc, scripts, entry)
         popen.assert_called_once_with(
             [
-                "yes | /some/cellxgene launch /tmp/czi/pbmc3k.h5ad --port 8000 --host 127.0.0.1 --disable-annotations --scripts http://example.com/script.js --scripts http://example.com/script2.js"
+                "yes | /some/cellxgene launch /tmp/czi/pbmc3k.h5ad --port 8000 --host 127.0.0.1 --disable-annotations --disable-gene-sets-save --scripts http://example.com/script.js --scripts http://example.com/script2.js"
             ],
             shell=True,
             stderr=-1,
             stdout=-1,
         )
         self.assertEqual("An unexpected error", context.exception.stderr)
+
+    @patch("subprocess.Popen")
+    def test_launch_GIVEN_annotations_enabled_THEN_set_flags(self, popen):
+        subprocess = MagicMock()
+        subprocess.stdout.readline().decode.return_value = (
+            "[cellxgene] Type CTRL-C at any time to exit.\n"
+        )
+        subprocess.stderr.read().decode.return_value = ""
+        popen.return_value = subprocess
+
+        key = CacheKey(
+            FileItem("/czi/", name="pbmc3k.h5ad", type=ItemType.h5ad),
+            FileItemSource("/tmp", "local"),
+            FileItem(
+                "/czi/pbmc3k_annotations/", name="foo.csv", type=ItemType.annotation
+            ),
+        )
+        entry = CacheEntry.for_key(key, 8000)
+        import cellxgene_gateway.subprocess_backend
+
+        cellxgene_gateway.subprocess_backend.enable_annotations = True
+        try:
+            backend = cellxgene_gateway.subprocess_backend.SubprocessBackend()
+            cellxgene_loc = "/some/cellxgene"
+
+            backend.launch(cellxgene_loc, [], entry)
+        finally:
+            cellxgene_gateway.subprocess_backend.enable_annotations = False
+        popen.assert_called_once_with(
+            [
+                "yes | /some/cellxgene launch /tmp/czi/pbmc3k.h5ad --port 8000 --host 127.0.0.1 --annotations-file /tmp/czi/pbmc3k_annotations/foo.csv --gene-sets-file /tmp/czi/pbmc3k_annotations/foo_gene_sets.csv"
+            ],
+            shell=True,
+            stderr=-1,
+            stdout=-1,
+        )