#73 moved new annotation link to front

68 read from subprocess

Changelog.md

@@ -1,3 +1,10 @@
+# 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)

README.md

@@ -73,9 +73,11 @@ 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_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.
+* `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.

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.9"
+__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"
@@ -111,7 +112,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 +170,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/filecrawl.py

@@ -7,7 +7,7 @@
 # OR CONDITIONS OF ANY KIND, either express or implied. See the License for
 # the specific language governing permissions and limitations under the License.
 
-import os
+import html
 import urllib.parse
 
 from cellxgene_gateway import env, flask_util
@@ -19,19 +19,17 @@ def render_annotations(item, item_source):
     url = flask_util.view_url(
         item_source.get_annotations_subpath(item), item_source.name
     )
-    new_annotation = f"<a class='new' href='{url}'>new</a>"
+    new_annotation = [f"<a class='new' href='{url}'>new</a>"]
+
     annotations = (
-        ", ".join(
-            [
-                f"<a href='{CacheKey(item, item_source, a).view_url}/'>{a.name}</a>"
-                for a in item.annotations
-            ]
-        )
-        + ", "
+        [
+            f"<a href='{CacheKey(item, item_source, a).view_url}/'>{html.escape(a.name)}</a>"
+            for a in item.annotations
+        ]
         if item.annotations
-        else ""
+        else []
     )
-    return " | annotations: " + annotations + new_annotation
+    return "| annotations: " + ", ".join(new_annotation + annotations)
 
 
 def render_item(item, item_source):

cellxgene_gateway/gateway.py

@@ -299,7 +299,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/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

@@ -17,6 +17,8 @@ from cellxgene_gateway.dir_util import make_annotations
 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):
@@ -55,7 +57,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 +86,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}")

tests/test_filecrawl.py

@@ -13,26 +13,76 @@ from cellxgene_gateway.items.item import ItemTree, ItemType
 source = FileItemSource("/tmp")
 
 
+def make_entry(subpath="somepath", annotations=None):
+    return FileItem(
+        subpath=subpath,
+        name="entry",
+        ext=".h5ad",
+        type=ItemType.h5ad,
+        annotations=annotations,
+    )
+
+
 class TestRenderEntry(unittest.TestCase):
     def test_GIVEN_path_both_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="/somepath/", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="/somepath/")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
 
     def test_GIVEN_path_starts_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="/somepath", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="/somepath")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
 
     def test_GIVEN_path_ends_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="somepath/", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="somepath/")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
 
     def test_GIVEN_path_no_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="somepath", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="somepath")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
+
+
+class TestRenderAnnotation(unittest.TestCase):
+    def test_GIVEN_no_annotation_THEN_new_alone(self):
+        entry = make_entry(annotations=None)
+        rendered = render_item(entry, source)
+        self.assertIn(
+            "> | annotations: <a class='new' href='/source/Files:/tmp/view/somepath/entry_annotations'>new</a></li>",
+            rendered,
+        )
+
+    def test_GIVEN_annotation_THEN_new_before(self):
+        annotation = FileItem(
+            subpath="somepath/entry_annotations",
+            name="annot",
+            ext=".csv",
+            type=ItemType.annotation,
+        )
+        entry = make_entry(annotations=[annotation])
+        rendered = render_item(entry, source)
+        self.assertIn(
+            "> | annotations: <a class='new' href='/source/Files:/tmp/view/somepath/entry_annotations'>new</a>,"
+            " <a href='/source/Files:/tmp/view/somepath/entry_annotations/annot.csv/'>annot</a></li>",
+            rendered,
+        )
+
+    def test_GIVEN_annotation_THEN_escaped(self):
+        annotation = FileItem(
+            subpath="somepath/entry_annotations",
+            name="hot&cold",
+            ext=".csv",
+            type=ItemType.annotation,
+        )
+        entry = make_entry(annotations=[annotation])
+        rendered = render_item(entry, source)
+        self.assertIn(
+            "> | annotations: <a class='new' href='/source/Files:/tmp/view/somepath/entry_annotations'>new</a>,"
+            " <a href='/source/Files:/tmp/view/somepath/entry_annotations/hot%26cold.csv/'>hot&amp;cold</a></li>",
+            rendered,
+        )
 
 
 class TestRenderItemSource(unittest.TestCase):

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):