From b689357f2989bfb296099cf42d534226c2e7e5bd Mon Sep 17 00:00:00 2001 From: Alok Saldanha Date: Sun, 30 Aug 2020 13:36:39 -0400 Subject: [PATCH 1/2] Applied "Formatting and new enumeration for Cellxgene-gateway" patch --- cellxgene_gateway/backend_cache.py | 10 +- cellxgene_gateway/cache_entry.py | 94 ++++++++-------- cellxgene_gateway/cache_key.py | 5 +- cellxgene_gateway/dir_util.py | 11 +- cellxgene_gateway/env.py | 32 ++++-- cellxgene_gateway/extra_scripts.py | 3 +- cellxgene_gateway/filecrawl.py | 54 +++++++-- cellxgene_gateway/gateway.py | 105 ++++++++++++------ cellxgene_gateway/path_util.py | 28 +++-- cellxgene_gateway/prune_process_cache.py | 28 +++-- cellxgene_gateway/subprocess_backend.py | 21 +++- cellxgene_gateway/templates/cache_status.html | 4 +- environment.yml | 2 +- setup.py | 21 ++-- tests/test_cache_entry.py | 13 +++ tests/test_dir_util.py | 38 ------- tests/test_extra_scripts.py | 19 ++-- tests/test_filecrawl.py | 46 ++++++++ tests/test_prune_process_cache.py | 17 +-- 19 files changed, 358 insertions(+), 193 deletions(-) create mode 100644 tests/test_cache_entry.py delete mode 100644 tests/test_dir_util.py create mode 100644 tests/test_filecrawl.py diff --git a/cellxgene_gateway/backend_cache.py b/cellxgene_gateway/backend_cache.py index 31dd4a3..df20eb2 100644 --- a/cellxgene_gateway/backend_cache.py +++ b/cellxgene_gateway/backend_cache.py @@ -13,7 +13,7 @@ from threading import Thread from flask_api import status from cellxgene_gateway import env -from cellxgene_gateway.cache_entry import CacheEntry +from cellxgene_gateway.cache_entry import CacheEntry, CacheEntryStatus from cellxgene_gateway.cellxgene_exception import CellxgeneException from cellxgene_gateway.subprocess_backend import SubprocessBackend @@ -22,8 +22,10 @@ process_backend = SubprocessBackend() def is_port_in_use(port): import socket + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: - return s.connect_ex(('localhost', port)) == 0 + return s.connect_ex(("localhost", port)) == 0 + class BackendCache: def __init__(self): @@ -38,7 +40,9 @@ class BackendCache: matches = [ c for c in contents - if c.key.dataset == key.dataset and c.key.annotation_file == key.annotation_file and c.status != "terminated" + if c.key.dataset == key.dataset + and c.key.annotation_file == key.annotation_file + and c.status != CacheEntryStatus.terminated ] if len(matches) == 0: diff --git a/cellxgene_gateway/cache_entry.py b/cellxgene_gateway/cache_entry.py index 2b1a429..09108c9 100644 --- a/cellxgene_gateway/cache_entry.py +++ b/cellxgene_gateway/cache_entry.py @@ -6,17 +6,25 @@ # under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES # OR CONDITIONS OF ANY KIND, either express or implied. See the License for # the specific language governing permissions and limitations under the License. -import psutil -import logging import datetime +import logging -from flask import make_response, request, render_template +import psutil +from enum import Enum +from flask import make_response, render_template, request from requests import get, post, put from cellxgene_gateway import env from cellxgene_gateway.cellxgene_exception import CellxgeneException -from cellxgene_gateway.util import current_time_stamp from cellxgene_gateway.flask_util import querystring +from cellxgene_gateway.util import current_time_stamp + + +class CacheEntryStatus(Enum): + loaded = "loaded" + loading = "loading" + error = "error" + terminated = "terminated" class CacheEntry: def __init__( @@ -26,7 +34,7 @@ class CacheEntry: port, launchtime, timestamp, - status, + status: CacheEntryStatus, message, all_output, stderr, @@ -52,7 +60,7 @@ class CacheEntry: port, current_time_stamp(), current_time_stamp(), - "loading", + CacheEntryStatus.loading, None, None, None, @@ -61,13 +69,13 @@ class CacheEntry: def set_loaded(self, pid): self.pid = pid - self.status = "loaded" + self.status = CacheEntryStatus.loaded def set_error(self, message, stderr, http_status): self.message = message self.stderr = stderr self.http_status = http_status - self.status = "error" + self.status = CacheEntryStatus.error def append_output(self, output): if self.all_output == None: @@ -77,10 +85,12 @@ class CacheEntry: def terminate(self): pid = self.pid - if pid != None and self.status != "terminated": + if pid != None and self.status != CacheEntryStatus.terminated: terminated = [] + def on_terminate(p): terminated.append(p.pid) + p = psutil.Process(pid) children = p.children() for child in children: @@ -89,44 +99,46 @@ class CacheEntry: terminated.append(p.pid) p.terminate() psutil.wait_procs([p], callback=on_terminate) - logging.getLogger("cellxgene_gateway").info(f"terminated {terminated}") - self.status = "terminated" + logging.getLogger("cellxgene_gateway").info( + f"terminated {terminated}" + ) + self.status = CacheEntryStatus.terminated def serve_content(self, path): - gateway_basepath = ( - f"{env.external_protocol}://{env.external_host}/view/{self.key.pathpart}/" - ) + gateway_basepath = f"{env.external_protocol}://{env.external_host}/view/{self.key.pathpart}/" subpath = path[len(self.key.pathpart) :] # noqa: E203 - + if len(subpath) == 0: r = make_response(f"Redirect to {gateway_basepath}\n", 301) - r.headers["location"] = gateway_basepath+querystring() + r.headers["location"] = gateway_basepath + querystring() return r - elif self.status == "loading": + elif self.status == CacheEntryStatus.loading: launch_time = datetime.datetime.fromtimestamp(self.launchtime) return render_template( - "loading.html", launchtime=launch_time, all_output=self.all_output + "loading.html", + launchtime=launch_time, + all_output=self.all_output, ) port = self.port cellxgene_basepath = f"http://127.0.0.1:{port}" headers = {} copy_headers = [ - 'accept', - 'accept-encoding', - 'accept-language', - 'cache-control', - 'connection', - 'content-length', - 'content-type', - 'cookie', - 'host', - 'origin', - 'pragma', - 'referer', - 'sec-fetch-mode', - 'sec-fetch-site', - 'user-agent' + "accept", + "accept-encoding", + "accept-language", + "cache-control", + "connection", + "content-length", + "content-type", + "cookie", + "host", + "origin", + "pragma", + "referer", + "sec-fetch-mode", + "sec-fetch-site", + "user-agent", ] for h in copy_headers: if h in request.headers: @@ -135,20 +147,14 @@ class CacheEntry: full_path = cellxgene_basepath + subpath + querystring() if request.method in ["GET", "HEAD", "OPTIONS"]: - cellxgene_response = get( - full_path, headers=headers - ) + cellxgene_response = get(full_path, headers=headers) elif request.method == "PUT": cellxgene_response = put( - full_path, - headers=headers, - data=request.data, + full_path, headers=headers, data=request.data, ) elif request.method == "POST": cellxgene_response = post( - full_path, - headers=headers, - data=request.data, + full_path, headers=headers, data=request.data, ) else: raise CellxgeneException( @@ -169,9 +175,7 @@ class CacheEntry: resp_headers[h] = cellxgene_response.headers[h] gateway_response = make_response( - gateway_content, - cellxgene_response.status_code, - resp_headers, + gateway_content, cellxgene_response.status_code, resp_headers, ) return gateway_response diff --git a/cellxgene_gateway/cache_key.py b/cellxgene_gateway/cache_key.py index 72c1810..13f069b 100644 --- a/cellxgene_gateway/cache_key.py +++ b/cellxgene_gateway/cache_key.py @@ -17,11 +17,12 @@ from cellxgene_gateway.cellxgene_exception import CellxgeneException # There are three kinds of CacheKey: # 1) somedir/dataset.h5ad: a dataset # in this case, pathpart == dataset == 'somedir/dataset.h5ad' -# 2) somedir/dataset_annotations/saldaal1-T5HMVBNV.csv : an actual annotaitons file. -# in this case, pathpart == 'dataset_annotations/saldaal1-T5HMVBNV.csv', dataset == 'somedir/dataset.h5ad' +# 2) somedir/dataset_annotations/my_annotations.csv : an actual annotaitons file. +# in this case, pathpart == 'dataset_annotations/my_annotations.csv', dataset == 'somedir/dataset.h5ad' # 3) somedir/dataset_annotations: an annotation directory. The corresponding h5ad must exist, but the directory may not. # in this case, pathpart == 'dataset_annotations', dataset == 'somedir/dataset.h5ad' + class CacheKey: def __init__(self, pathpart, dataset, annotation_file): self.pathpart = pathpart diff --git a/cellxgene_gateway/dir_util.py b/cellxgene_gateway/dir_util.py index 4ddc1c7..a7bc262 100644 --- a/cellxgene_gateway/dir_util.py +++ b/cellxgene_gateway/dir_util.py @@ -51,8 +51,13 @@ def create_dir(parent_path, dir_name): else: os.mkdir(full_path) -annotations_suffix = '_annotations' + +annotations_suffix = "_annotations" + + def make_h5ad(el): - return el[:-len(annotations_suffix)]+'.h5ad' + return el[: -len(annotations_suffix)] + ".h5ad" + + def make_annotations(el): - return el[:-5]+annotations_suffix + return el[:-5] + annotations_suffix diff --git a/cellxgene_gateway/env.py b/cellxgene_gateway/env.py index 22206d3..5650cb1 100644 --- a/cellxgene_gateway/env.py +++ b/cellxgene_gateway/env.py @@ -7,21 +7,32 @@ # 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 logging +import os import socket cellxgene_location = os.environ.get("CELLXGENE_LOCATION") cellxgene_data = os.environ.get("CELLXGENE_DATA") gateway_port = int(os.environ.get("GATEWAY_PORT", "5005")) -external_host = os.environ.get("EXTERNAL_HOST", os.environ.get("GATEWAY_HOST", f"localhost:{gateway_port}")) -external_protocol = os.environ.get("EXTERNAL_PROTOCOL", os.environ.get("GATEWAY_PROTOCOL", "http")) +external_host = os.environ.get( + "EXTERNAL_HOST", + os.environ.get("GATEWAY_HOST", f"localhost:{gateway_port}"), +) +external_protocol = os.environ.get( + "EXTERNAL_PROTOCOL", os.environ.get("GATEWAY_PROTOCOL", "http") +) ip = os.environ.get("GATEWAY_IP") extra_scripts = os.environ.get("GATEWAY_EXTRA_SCRIPTS") ttl = os.environ.get("GATEWAY_TTL") -enable_upload = os.environ.get("GATEWAY_ENABLE_UPLOAD", "").lower() in ['true', '1'] -enable_annotations = os.environ.get("GATEWAY_ENABLE_ANNOTATIONS", "").lower() in ['true', '1'] -enable_backed_mode = os.environ.get("GATEWAY_ENABLE_BACKED_MODE", "").lower() in ['true', '1'] +enable_upload = os.environ.get( + "GATEWAY_ENABLE_UPLOAD", "" +).lower() in ["true", "1"] +enable_annotations = os.environ.get( + "GATEWAY_ENABLE_ANNOTATIONS", "" +).lower() in ["true", "1"] +enable_backed_mode = os.environ.get( + "GATEWAY_ENABLE_BACKED_MODE", "" +).lower() in ['true', '1'] env_vars = { "CELLXGENE_LOCATION": cellxgene_location, @@ -40,6 +51,7 @@ optional_env_vars = { "GATEWAY_ENABLE_BACKED_MODE": enable_backed_mode, } + def validate(): if not all(env_vars.values()): raise ValueError( @@ -58,5 +70,9 @@ def validate(): """ ) else: - logging.getLogger("cellxgene_gateway").info(f"Got required env: {env_vars}", ) - logging.getLogger("cellxgene_gateway").info(f"Got optional env: {optional_env_vars}") + logging.getLogger("cellxgene_gateway").info( + f"Got required env: {env_vars}", + ) + logging.getLogger("cellxgene_gateway").info( + f"Got optional env: {optional_env_vars}" + ) diff --git a/cellxgene_gateway/extra_scripts.py b/cellxgene_gateway/extra_scripts.py index 7ef49e5..abf2218 100644 --- a/cellxgene_gateway/extra_scripts.py +++ b/cellxgene_gateway/extra_scripts.py @@ -7,9 +7,10 @@ # OR CONDITIONS OF ANY KIND, either express or implied. See the License for # the specific language governing permissions and limitations under the License. -from cellxgene_gateway import env from json import loads +from cellxgene_gateway import env + def get_extra_scripts(): # can be array of script tags to inject on every page, e.g. for google analytics could be diff --git a/cellxgene_gateway/filecrawl.py b/cellxgene_gateway/filecrawl.py index b1fa7cc..c505f66 100644 --- a/cellxgene_gateway/filecrawl.py +++ b/cellxgene_gateway/filecrawl.py @@ -19,20 +19,40 @@ def recurse_dir(path): all_entries = sorted(os.listdir(path)) def is_h5ad(el): - return el.endswith('.h5ad') and os.path.isfile(os.path.join(path, el)) + return el.endswith(".h5ad") and os.path.isfile(os.path.join(path, el)) + h5ad_entries = [x for x in all_entries if is_h5ad(x)] - annotation_dir_entries = [x for x in all_entries if x.endswith(annotations_suffix) and make_h5ad(x) in h5ad_entries] + annotation_dir_entries = [ + x + for x in all_entries + if x.endswith(annotations_suffix) and make_h5ad(x) in h5ad_entries + ] + def list_annotations(el): full_path = os.path.join(path, el) if not os.path.isdir(full_path): entries = [] else: - entries = [{ - "name": x[:-13] if (len(x) > 13 and x[-13] in ['-','_']) else ( - x[:-4] if x.endswith('.csv') else x), - "path": os.path.join(full_path, x).replace(env.cellxgene_data, ""), - } for x in sorted(os.listdir(full_path)) if x.endswith('.csv') and os.path.isfile(os.path.join(full_path, x))] - return [{"name":'new', "class":'new', "path":full_path.replace(env.cellxgene_data, "")}] + entries + entries = [ + { + "name": x[:-13] + if (len(x) > 13 and x[-13] in ["-", "_"]) + else (x[:-4] if x.endswith(".csv") else x), + "path": os.path.join(full_path, x).replace( + env.cellxgene_data, "" + ), + } + for x in sorted(os.listdir(full_path)) + if x.endswith(".csv") + and os.path.isfile(os.path.join(full_path, x)) + ] + return [ + { + "name": "new", + "class": "new", + "path": full_path.replace(env.cellxgene_data, ""), + } + ] + entries def make_entry(el): full_path = os.path.join(path, el) @@ -63,16 +83,26 @@ def recurse_dir(path): def render_entries(entries): return "" + def get_url(entry): return f"/view/{ entry['path'].lstrip('/') }" + + def get_class(entry): - return f" class='{entry['class']}'" if 'class' in entry else '' + return f" class='{entry['class']}'" if "class" in entry else "" + def render_annotations(entry): - if len(entry['annotations']) > 0: - return ' | annotations: ' + ", ".join([f"{a['name']}" for a in entry['annotations']]) + if len(entry["annotations"]) > 0: + return " | annotations: " + ", ".join( + [ + f"{a['name']}" + for a in entry["annotations"] + ] + ) else: - return '' + return "" + def render_entry(entry): if entry["type"] == "file": diff --git a/cellxgene_gateway/gateway.py b/cellxgene_gateway/gateway.py index e98b569..227ee4b 100644 --- a/cellxgene_gateway/gateway.py +++ b/cellxgene_gateway/gateway.py @@ -7,16 +7,17 @@ # OR CONDITIONS OF ANY KIND, either express or implied. See the License for # the specific language governing permissions and limitations under the License. +import json +import logging + # import BaseHTTPServer import os -import logging -from threading import Thread, Lock -import json +from threading import Lock, Thread from flask import ( Flask, - redirect, make_response, + redirect, render_template, request, send_from_directory, @@ -27,22 +28,27 @@ from werkzeug.utils import secure_filename from cellxgene_gateway import env from cellxgene_gateway.backend_cache import BackendCache +from cellxgene_gateway.cache_entry import CacheEntryStatus from cellxgene_gateway.cellxgene_exception import CellxgeneException from cellxgene_gateway.dir_util import create_dir, is_subdir -from cellxgene_gateway.filecrawl import recurse_dir, render_entries from cellxgene_gateway.extra_scripts import get_extra_scripts +from cellxgene_gateway.filecrawl import recurse_dir, render_entries +from cellxgene_gateway.path_util import get_key from cellxgene_gateway.process_exception import ProcessException from cellxgene_gateway.prune_process_cache import PruneProcessCache from cellxgene_gateway.util import current_time_stamp -from cellxgene_gateway.path_util import get_key app = Flask(__name__) + def _force_https(app): def wrapper(environ, start_response): - environ['wsgi.url_scheme'] = env.external_protocol + environ["wsgi.url_scheme"] = env.external_protocol return app(environ, start_response) + return wrapper + + app.wsgi_app = _force_https(app.wsgi_app) cache = BackendCache() @@ -114,6 +120,7 @@ def index(): enable_upload=env.enable_upload, ) + def make_user(): dir_name = request.form["directory"] @@ -135,13 +142,17 @@ def upload_file(): upload_dir = request.form["path"] full_upload_path = os.path.join(env.cellxgene_data, upload_dir) - if is_subdir(full_upload_path, env.cellxgene_data) and os.path.isdir(full_upload_path): + if is_subdir(full_upload_path, env.cellxgene_data) and os.path.isdir( + full_upload_path + ): if request.method == "POST": if "file" in request.files: f = request.files["file"] if f and f.filename.endswith(".h5ad"): f.save( - os.path.join(full_upload_path, secure_filename(f.filename)) + os.path.join( + full_upload_path, secure_filename(f.filename) + ) ) return redirect("/filecrawl.html", code=302) else: @@ -163,31 +174,40 @@ def upload_file(): if env.enable_upload: - app.add_url_rule('/make_user', 'make_user', make_user, methods=["POST"]) - app.add_url_rule('/make_subdir', 'make_subdir', make_subdir, methods=["POST"]) - app.add_url_rule('/upload_file', 'upload_file', upload_file, methods=["POST"]) + app.add_url_rule("/make_user", "make_user", make_user, methods=["POST"]) + app.add_url_rule( + "/make_subdir", "make_subdir", make_subdir, methods=["POST"] + ) + app.add_url_rule( + "/upload_file", "upload_file", upload_file, methods=["POST"] + ) + @app.route("/filecrawl.html") def filecrawl(): entries = recurse_dir(env.cellxgene_data) rendered_html = render_entries(entries) - resp = make_response(render_template( - "filecrawl.html", - extra_scripts=get_extra_scripts(), - rendered_html=rendered_html, - )) + resp = make_response( + render_template( + "filecrawl.html", + extra_scripts=get_extra_scripts(), + rendered_html=rendered_html, + ) + ) resp.headers["Cache-Control"] = "no-cache, no-store, must-revalidate" resp.headers["Pragma"] = "no-cache" resp.headers["Expires"] = "0" - resp.headers['Cache-Control'] = 'public, max-age=0' + resp.headers["Cache-Control"] = "public, max-age=0" return resp + @app.route("/filecrawl/") def do_filecrawl(path): filecrawl_path = os.path.join(env.cellxgene_data, path) if not os.path.isdir(filecrawl_path): raise CellxgeneException( - "Path is not directory: " + filecrawl_path, status.HTTP_400_BAD_REQUEST + "Path is not directory: " + filecrawl_path, + status.HTTP_400_BAD_REQUEST, ) entries = recurse_dir(filecrawl_path) rendered_html = render_entries(entries) @@ -198,11 +218,16 @@ def do_filecrawl(path): path=path, ) + entry_lock = Lock() + + @app.route("/view/", methods=["GET", "PUT", "POST"]) def do_view(path): key = get_key(path) - print(f"view path={path}, dataset={key.dataset}, annotation_file= {key.annotation_file}, key={key.pathpart}") + print( + f"view path={path}, dataset={key.dataset}, annotation_file= {key.annotation_file}, key={key.pathpart}" + ) with entry_lock: match = cache.check_entry(key) if match is None: @@ -211,9 +236,9 @@ def do_view(path): match.timestamp = current_time_stamp() - if match.status == "loaded" or match.status == "loading": + if match.status == CacheEntryStatus.loaded or match.status == CacheEntryStatus.loading: return match.serve_content(path) - elif match.status == "error": + elif match.status == CacheEntryStatus.error: raise ProcessException.from_cache_entry(match) @@ -221,16 +246,25 @@ def do_view(path): def do_GET_status(): return render_template("cache_status.html", entry_list=cache.entry_list) + @app.route("/cache_status.json", methods=["GET"]) def do_GET_status_json(): - return json.dumps({'launchtime':app.launchtime, - 'entry_list':[{ - 'dataset': entry.key.dataset, - 'annotation_file': entry.key.annotation_file, - 'launchtime': entry.launchtime, - 'last_access': entry.timestamp, - 'status': entry.status - } for entry in cache.entry_list]}) + return json.dumps( + { + "launchtime": app.launchtime, + "entry_list": [ + { + "dataset": entry.key.dataset, + "annotation_file": entry.key.annotation_file, + "launchtime": entry.launchtime, + "last_access": entry.timestamp, + "status": entry.status, + } + for entry in cache.entry_list + ], + } + ) + @app.route("/relaunch/", methods=["GET"]) def do_relaunch(path): @@ -239,7 +273,11 @@ def do_relaunch(path): if not match is None: match.terminate() qs = request.query_string.decode() - return redirect(url_for("do_view", path=path) + (f'?{qs}' if len(qs) > 0 else ''), code=302) + return redirect( + url_for("do_view", path=path) + (f"?{qs}" if len(qs) > 0 else ""), + code=302, + ) + @app.route("/terminate/", methods=["GET"]) def do_terminate(path): @@ -251,7 +289,10 @@ def do_terminate(path): def main(): - logging.basicConfig(level=logging.INFO, format='%(asctime)s:%(name)s:%(levelname)s:%(message)s') + logging.basicConfig( + level=logging.INFO, + format="%(asctime)s:%(name)s:%(levelname)s:%(message)s", + ) env.validate() pruner = PruneProcessCache(cache) diff --git a/cellxgene_gateway/path_util.py b/cellxgene_gateway/path_util.py index 08e5ccf..1b23bc8 100644 --- a/cellxgene_gateway/path_util.py +++ b/cellxgene_gateway/path_util.py @@ -12,9 +12,10 @@ import os from flask_api import status from cellxgene_gateway import env +from cellxgene_gateway.cache_key import CacheKey from cellxgene_gateway.cellxgene_exception import CellxgeneException from cellxgene_gateway.dir_util import make_h5ad -from cellxgene_gateway.cache_key import CacheKey + def get_key(path): if path == "/" or path == "": @@ -25,33 +26,35 @@ def get_key(path): trimmed = path[:-1] if path[-1] == "/" else path try: # valid paths come in three forms: - if trimmed.endswith('.h5ad') and data_file_exists(trimmed): + if trimmed.endswith(".h5ad") and data_file_exists(trimmed): # 1) somedir/dataset.h5ad: a dataset return CacheKey(trimmed, trimmed, None) - elif trimmed.endswith('.csv'): + elif trimmed.endswith(".csv"): - # 2) somedir/dataset_annotations/saldaal1-T5HMVBNV.csv : an actual annotations file. + # 2) somedir/dataset_annotations/my_annotations.csv : an actual annotations file. annotations_dir = os.path.split(trimmed)[0] dataset = make_h5ad(annotations_dir) if data_file_exists(dataset): data_dir_ensure(annotations_dir) return CacheKey(trimmed, dataset, trimmed) - elif trimmed.endswith('_annotations') and data_dir_exists(trimmed): + elif trimmed.endswith("_annotations") and data_dir_exists(trimmed): # 3) somedir/dataset_annotations: an annotation directory. The corresponding h5ad must exist, but the directory may not. dataset = make_h5ad(trimmed) if data_file_exists(dataset): - return CacheKey(trimmed, dataset, '') + return CacheKey(trimmed, dataset, "") except CellxgeneException: pass split = os.path.split(trimmed) return get_key(split[0]) + def validate_exists(file_path): if not os.path.exists(file_path): raise CellxgeneException( "File does not exist: " + file_path, status.HTTP_400_BAD_REQUEST ) + def validate_is_file(file_path): validate_exists(file_path) if not os.path.isfile(file_path): @@ -59,6 +62,8 @@ def validate_is_file(file_path): "Path is not file: " + file_path, status.HTTP_400_BAD_REQUEST ) return + + def validate_is_dir(file_path): validate_exists(file_path) if not os.path.isdir(file_path): @@ -67,29 +72,36 @@ def validate_is_dir(file_path): ) return + def data_file_exists(dataset): file_path = os.path.join(env.cellxgene_data, dataset) validate_is_file(file_path) return True + + def data_dir_exists(dataset): file_path = os.path.join(env.cellxgene_data, dataset) validate_is_dir(file_path) return True + + def data_dir_ensure(dataset): file_path = os.path.join(env.cellxgene_data, dataset) if not os.path.exists(file_path): os.makedirs(file_path) + def get_file_path(key): dataset = key.dataset file_path = os.path.join(env.cellxgene_data, dataset) validate_is_file(file_path) return file_path + def get_annotation_file_path(key): if key.annotation_file is None: return None - if key.annotation_file == '': - return '' + if key.annotation_file == "": + return "" file_path = os.path.join(env.cellxgene_data, key.annotation_file) return file_path diff --git a/cellxgene_gateway/prune_process_cache.py b/cellxgene_gateway/prune_process_cache.py index 3bb8f2b..f332ffd 100644 --- a/cellxgene_gateway/prune_process_cache.py +++ b/cellxgene_gateway/prune_process_cache.py @@ -7,17 +7,17 @@ # OR CONDITIONS OF ANY KIND, either express or implied. See the License for # the specific language governing permissions and limitations under the License. -import time import logging +import time -from cellxgene_gateway.util import current_time_stamp from cellxgene_gateway.env import ttl +from cellxgene_gateway.util import current_time_stamp class PruneProcessCache: def __init__(self, cache): self.cache = cache - self.expire_seconds = (3600 if ttl is None else int(ttl)) + self.expire_seconds = 3600 if ttl is None else int(ttl) def __call__(self): while True: @@ -27,14 +27,24 @@ class PruneProcessCache: def prune(self): timestamp = current_time_stamp() cutoff = timestamp - self.expire_seconds - processes_to_delete = [p for p in self.cache.entry_list if p.timestamp < cutoff] - processes_to_keep = [p for p in self.cache.entry_list if not p.timestamp < cutoff] + processes_to_delete = [ + p for p in self.cache.entry_list if p.timestamp < cutoff + ] + processes_to_keep = [ + p for p in self.cache.entry_list if not p.timestamp < cutoff + ] logger = logging.getLogger("cellxgene_gateway") - logger.debug(f"Cutoff {cutoff} = timestamp {timestamp} - expire seconds {self.expire_seconds} , keeping {processes_to_keep}") - + logger.debug( + f"Cutoff {cutoff} = timestamp {timestamp} - expire seconds {self.expire_seconds} , keeping {processes_to_keep}" + ) + for process in processes_to_delete: try: - logger.info(f"pruning process {process.pid} ({process.key.dataset})") + logger.info( + f"pruning process {process.pid} ({process.key.dataset})" + ) self.cache.prune(process) except Exception: - logger.exception("failed to prune process {process.pid} ({process.dataset})") + logger.exception( + "failed to prune process {process.pid} ({process.dataset})" + ) diff --git a/cellxgene_gateway/subprocess_backend.py b/cellxgene_gateway/subprocess_backend.py index 072c680..9c793b5 100644 --- a/cellxgene_gateway/subprocess_backend.py +++ b/cellxgene_gateway/subprocess_backend.py @@ -11,17 +11,22 @@ import logging import subprocess from flask_api import status -from cellxgene_gateway.env import enable_annotations, enable_backed_mode -from cellxgene_gateway.process_exception import ProcessException + +from cellxgene_gateway.cache_entry import CacheEntryStatus from cellxgene_gateway.dir_util import make_annotations -from cellxgene_gateway.path_util import get_file_path, get_annotation_file_path +from cellxgene_gateway.env import enable_annotations, enable_backed_mode +from cellxgene_gateway.path_util import get_annotation_file_path, get_file_path +from cellxgene_gateway.process_exception import ProcessException + class SubprocessBackend: def __init__(self): pass - def create_cmd(self, cellxgene_loc, file_path, port, scripts, annotation_file_path): + def create_cmd( + self, cellxgene_loc, file_path, port, scripts, annotation_file_path + ): if enable_annotations and not annotation_file_path is None: if annotation_file_path == "": extra_args = f" --annotations-dir {make_annotations(file_path)}" @@ -47,7 +52,11 @@ class SubprocessBackend: def launch(self, cellxgene_loc, scripts, cache_entry): cmd = self.create_cmd( - cellxgene_loc, get_file_path(cache_entry.key), cache_entry.port, scripts, get_annotation_file_path(cache_entry.key) + cellxgene_loc, + get_file_path(cache_entry.key), + cache_entry.port, + scripts, + get_annotation_file_path(cache_entry.key), ) logging.getLogger("cellxgene_gateway").info(f"launching {cmd}") process = subprocess.Popen( @@ -70,7 +79,7 @@ class SubprocessBackend: message = "Cellxgene failed to launch dataset." http_status = status.HTTP_500_INTERNAL_SERVER_ERROR - cache_entry.status = "error" + cache_entry.status = CacheEntryStatus.error cache_entry.set_error(message, stderr, http_status) raise ProcessException.from_cache_entry(cache_entry) diff --git a/cellxgene_gateway/templates/cache_status.html b/cellxgene_gateway/templates/cache_status.html index 34b6f52..956fe56 100644 --- a/cellxgene_gateway/templates/cache_status.html +++ b/cellxgene_gateway/templates/cache_status.html @@ -48,11 +48,11 @@ {{ entry.port }} {{ entry.launchtime }} {{ entry.timestamp }} - {{ entry.status }} + {{ entry.status.name }} {{ entry.message }} {{ entry.http_status }} - {% if entry.status == 'loaded' %} + {% if entry.status.name == 'loaded' %} terminate {% endif %} diff --git a/environment.yml b/environment.yml index 4063d64..912f142 100644 --- a/environment.yml +++ b/environment.yml @@ -1,4 +1,4 @@ -name: cellxgene-dev +name: cellxgene-gateway channels: - conda-forge dependencies: diff --git a/setup.py b/setup.py index 9a8dfb8..cfb38d4 100644 --- a/setup.py +++ b/setup.py @@ -3,22 +3,25 @@ import codecs from setuptools import find_packages, setup import sys -if sys.version_info < (3,6): - sys.exit('Sorry, Python < 3.6 is not supported') +if sys.version_info < (3, 6): + sys.exit("Sorry, Python < 3.6 is not supported") + def read(rel_path): here = os.path.abspath(os.path.dirname(__file__)) - with codecs.open(os.path.join(here, rel_path), 'r') as fp: + with codecs.open(os.path.join(here, rel_path), "r") as fp: return fp.read() + def get_version(rel_path): for line in read(rel_path).splitlines(): - if line.startswith('__version__'): + if line.startswith("__version__"): delim = '"' if '"' in line else "'" return line.split(delim)[1] else: raise RuntimeError("Unable to find version string.") + def parse_requirements(): reqs = [] with open("requirements.txt", "r") as f: @@ -26,6 +29,7 @@ def parse_requirements(): reqs.append(l.strip("\n")) return reqs + with open("README.md", "r") as fh: long_description = fh.read() @@ -50,13 +54,14 @@ setup( "cellxgene_gateway": [ "static/css/homepagestyle.css", "static/nibr.ico", - "templates/*.html" - ]}, - data_files=[('', ['README.md', 'LICENSE'])], + "templates/*.html", + ] + }, + data_files=[("", ["README.md", "LICENSE"])], install_requires=install_reqs, entry_points={ "console_scripts": ["cellxgene-gateway=cellxgene_gateway.gateway:main"] }, classifiers=["Topic :: Scientific/Engineering :: Visualization"], - python_requires='>=3.6', + python_requires=">=3.6", ) diff --git a/tests/test_cache_entry.py b/tests/test_cache_entry.py new file mode 100644 index 0000000..b3a3c4b --- /dev/null +++ b/tests/test_cache_entry.py @@ -0,0 +1,13 @@ +import unittest +from unittest.mock import MagicMock, patch + +from cellxgene_gateway.cache_entry import CacheEntry, CacheEntryStatus + + +class TestCacheEntry(unittest.TestCase): + def test_GIVEN_key_and_port_THEN_returns_loading_CacheEntry(self): + entry = CacheEntry.for_key("some-key", 1) + self.assertEqual(entry.status, CacheEntryStatus.loading) + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_dir_util.py b/tests/test_dir_util.py deleted file mode 100644 index 3a5e5dc..0000000 --- a/tests/test_dir_util.py +++ /dev/null @@ -1,38 +0,0 @@ -import unittest -from unittest.mock import MagicMock, patch -from cellxgene_gateway.dir_util import render_entry - -class TestRenderEntry(unittest.TestCase): - def test_GIVEN_path_both_slash_THEN_view_has_single_slash(self): - entry = { - "path": "/somepath/", - "name": "entry", - "type": "file", - } - rendered = render_entry(entry) - self.assertIn('view/somepath', rendered) - def test_GIVEN_path_starts_slash_THEN_view_has_single_slash(self): - entry = { - "path": "/somepath", - "name": "entry", - "type": "file", - } - rendered = render_entry(entry) - self.assertIn('view/somepath', rendered) - def test_GIVEN_path_ends_slash_THEN_view_has_single_slash(self): - entry = { - "path": "somepath/", - "name": "entry", - "type": "file", - } - rendered = render_entry(entry) - self.assertIn('view/somepath', rendered) - def test_GIVEN_path_no_slash_THEN_view_has_single_slash(self): - entry = { - "path": "somepath", - "name": "entry", - "type": "file", - } - rendered = render_entry(entry) - self.assertIn('view/somepath', rendered) - \ No newline at end of file diff --git a/tests/test_extra_scripts.py b/tests/test_extra_scripts.py index 4e63c89..c429c55 100644 --- a/tests/test_extra_scripts.py +++ b/tests/test_extra_scripts.py @@ -1,23 +1,26 @@ import unittest from unittest.mock import MagicMock, patch + from cellxgene_gateway.extra_scripts import get_extra_scripts + class TestExtraScripts(unittest.TestCase): - @patch('cellxgene_gateway.env.extra_scripts', new='["abc","def"]') + @patch("cellxgene_gateway.env.extra_scripts", new='["abc","def"]') def test_GIVEN_two_scripts_THEN_returns_two_strings(self): - self.assertEqual(get_extra_scripts(), ['abc', 'def']) + self.assertEqual(get_extra_scripts(), ["abc", "def"]) - @patch('cellxgene_gateway.env.extra_scripts', new='["abc", "def"]') + @patch("cellxgene_gateway.env.extra_scripts", new='["abc", "def"]') def test_GIVEN_two_scripts_space_THEN_returns_two_strings(self): - self.assertEqual(get_extra_scripts(), ['abc', 'def']) + self.assertEqual(get_extra_scripts(), ["abc", "def"]) - @patch('cellxgene_gateway.env.extra_scripts', new=None) + @patch("cellxgene_gateway.env.extra_scripts", new=None) def test_GIVEN_none_THEN_returns_empty_array(self): self.assertEqual(get_extra_scripts(), []) - @patch('cellxgene_gateway.env.extra_scripts', new='[]') + @patch("cellxgene_gateway.env.extra_scripts", new="[]") def test_GIVEN_empty_string_THEN_returns_empty_array(self): self.assertEqual(get_extra_scripts(), []) -if __name__ == '__main__': - unittest.main() \ No newline at end of file + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_filecrawl.py b/tests/test_filecrawl.py new file mode 100644 index 0000000..bf30259 --- /dev/null +++ b/tests/test_filecrawl.py @@ -0,0 +1,46 @@ +import unittest +from unittest.mock import MagicMock, patch + +from cellxgene_gateway.filecrawl import render_entry + + +class TestRenderEntry(unittest.TestCase): + def test_GIVEN_path_both_slash_THEN_view_has_single_slash(self): + entry = { + "path": "/somepath/", + "name": "entry", + "type": "file", + "annotations": [] + } + rendered = render_entry(entry) + self.assertIn("view/somepath", rendered) + + def test_GIVEN_path_starts_slash_THEN_view_has_single_slash(self): + entry = { + "path": "/somepath", + "name": "entry", + "type": "file", + "annotations": [] + } + rendered = render_entry(entry) + self.assertIn("view/somepath", rendered) + + def test_GIVEN_path_ends_slash_THEN_view_has_single_slash(self): + entry = { + "path": "somepath/", + "name": "entry", + "type": "file", + "annotations": [] + } + rendered = render_entry(entry) + self.assertIn("view/somepath", rendered) + + def test_GIVEN_path_no_slash_THEN_view_has_single_slash(self): + entry = { + "path": "somepath", + "name": "entry", + "type": "file", + "annotations": [] + } + rendered = render_entry(entry) + self.assertIn("view/somepath", rendered) diff --git a/tests/test_prune_process_cache.py b/tests/test_prune_process_cache.py index fe84677..3609e3a 100644 --- a/tests/test_prune_process_cache.py +++ b/tests/test_prune_process_cache.py @@ -1,13 +1,15 @@ import unittest from unittest.mock import MagicMock, patch -from cellxgene_gateway.cache_entry import CacheEntry + from cellxgene_gateway.backend_cache import BackendCache +from cellxgene_gateway.cache_entry import CacheEntry + class TestPruneProcessCache(unittest.TestCase): - @patch('cellxgene_gateway.util.current_time_stamp', new=lambda:0) - @patch('cellxgene_gateway.env.ttl', new='10') - @patch('cellxgene_gateway.cache_entry.CacheEntry') - @patch('cellxgene_gateway.cache_entry.CacheEntry') + @patch("cellxgene_gateway.util.current_time_stamp", new=lambda: 0) + @patch("cellxgene_gateway.env.ttl", 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): from cellxgene_gateway.prune_process_cache import PruneProcessCache @@ -22,5 +24,6 @@ class TestPruneProcessCache(unittest.TestCase): self.assertEqual(len(cache.entry_list), 1) self.assertEqual(cache.entry_list[0], new) -if __name__ == '__main__': - unittest.main() \ No newline at end of file + +if __name__ == "__main__": + unittest.main() From 9219336356f3912457d724c65241abd514e2d885 Mon Sep 17 00:00:00 2001 From: Alok Saldanha Date: Sun, 30 Aug 2020 13:38:35 -0400 Subject: [PATCH 2/2] blackened code --- cellxgene_gateway/cache_entry.py | 1 + cellxgene_gateway/env.py | 9 +++++---- cellxgene_gateway/filecrawl.py | 8 +++++++- cellxgene_gateway/flask_util.py | 3 ++- cellxgene_gateway/gateway.py | 5 ++++- cellxgene_gateway/subprocess_backend.py | 5 +++-- tests/test_cache_entry.py | 1 + tests/test_filecrawl.py | 8 ++++---- 8 files changed, 27 insertions(+), 13 deletions(-) diff --git a/cellxgene_gateway/cache_entry.py b/cellxgene_gateway/cache_entry.py index 09108c9..438d612 100644 --- a/cellxgene_gateway/cache_entry.py +++ b/cellxgene_gateway/cache_entry.py @@ -26,6 +26,7 @@ class CacheEntryStatus(Enum): error = "error" terminated = "terminated" + class CacheEntry: def __init__( self, diff --git a/cellxgene_gateway/env.py b/cellxgene_gateway/env.py index 5650cb1..dc72f1a 100644 --- a/cellxgene_gateway/env.py +++ b/cellxgene_gateway/env.py @@ -24,15 +24,16 @@ 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") -enable_upload = os.environ.get( - "GATEWAY_ENABLE_UPLOAD", "" -).lower() in ["true", "1"] +enable_upload = os.environ.get("GATEWAY_ENABLE_UPLOAD", "").lower() in [ + "true", + "1", +] enable_annotations = os.environ.get( "GATEWAY_ENABLE_ANNOTATIONS", "" ).lower() in ["true", "1"] enable_backed_mode = os.environ.get( "GATEWAY_ENABLE_BACKED_MODE", "" -).lower() in ['true', '1'] +).lower() in ["true", "1"] env_vars = { "CELLXGENE_LOCATION": cellxgene_location, diff --git a/cellxgene_gateway/filecrawl.py b/cellxgene_gateway/filecrawl.py index c505f66..cd265f6 100644 --- a/cellxgene_gateway/filecrawl.py +++ b/cellxgene_gateway/filecrawl.py @@ -9,7 +9,12 @@ import os from cellxgene_gateway import env -from cellxgene_gateway.dir_util import make_h5ad, make_annotations, annotations_suffix +from cellxgene_gateway.dir_util import ( + make_h5ad, + make_annotations, + annotations_suffix, +) + def recurse_dir(path): if not os.path.exists(path): @@ -18,6 +23,7 @@ def recurse_dir(path): ) all_entries = sorted(os.listdir(path)) + def is_h5ad(el): return el.endswith(".h5ad") and os.path.isfile(os.path.join(path, el)) diff --git a/cellxgene_gateway/flask_util.py b/cellxgene_gateway/flask_util.py index 112999f..9132929 100644 --- a/cellxgene_gateway/flask_util.py +++ b/cellxgene_gateway/flask_util.py @@ -9,6 +9,7 @@ from flask import request + def querystring(): qs = request.query_string.decode() - return f'?{qs}' if len(qs) > 0 else '' + return f"?{qs}" if len(qs) > 0 else "" diff --git a/cellxgene_gateway/gateway.py b/cellxgene_gateway/gateway.py index 227ee4b..c72b1b2 100644 --- a/cellxgene_gateway/gateway.py +++ b/cellxgene_gateway/gateway.py @@ -236,7 +236,10 @@ def do_view(path): match.timestamp = current_time_stamp() - if match.status == CacheEntryStatus.loaded or match.status == CacheEntryStatus.loading: + if ( + match.status == CacheEntryStatus.loaded + or match.status == CacheEntryStatus.loading + ): return match.serve_content(path) elif match.status == CacheEntryStatus.error: raise ProcessException.from_cache_entry(match) diff --git a/cellxgene_gateway/subprocess_backend.py b/cellxgene_gateway/subprocess_backend.py index 9c793b5..e68ec69 100644 --- a/cellxgene_gateway/subprocess_backend.py +++ b/cellxgene_gateway/subprocess_backend.py @@ -19,7 +19,6 @@ from cellxgene_gateway.path_util import get_annotation_file_path, get_file_path from cellxgene_gateway.process_exception import ProcessException - class SubprocessBackend: def __init__(self): pass @@ -29,7 +28,9 @@ class SubprocessBackend: ): if enable_annotations and not annotation_file_path is None: if annotation_file_path == "": - extra_args = f" --annotations-dir {make_annotations(file_path)}" + extra_args = ( + f" --annotations-dir {make_annotations(file_path)}" + ) else: extra_args = f" --annotations-file {annotation_file_path}" else: diff --git a/tests/test_cache_entry.py b/tests/test_cache_entry.py index b3a3c4b..c2563c7 100644 --- a/tests/test_cache_entry.py +++ b/tests/test_cache_entry.py @@ -9,5 +9,6 @@ class TestCacheEntry(unittest.TestCase): entry = CacheEntry.for_key("some-key", 1) self.assertEqual(entry.status, CacheEntryStatus.loading) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_filecrawl.py b/tests/test_filecrawl.py index bf30259..8e4fc48 100644 --- a/tests/test_filecrawl.py +++ b/tests/test_filecrawl.py @@ -10,7 +10,7 @@ class TestRenderEntry(unittest.TestCase): "path": "/somepath/", "name": "entry", "type": "file", - "annotations": [] + "annotations": [], } rendered = render_entry(entry) self.assertIn("view/somepath", rendered) @@ -20,7 +20,7 @@ class TestRenderEntry(unittest.TestCase): "path": "/somepath", "name": "entry", "type": "file", - "annotations": [] + "annotations": [], } rendered = render_entry(entry) self.assertIn("view/somepath", rendered) @@ -30,7 +30,7 @@ class TestRenderEntry(unittest.TestCase): "path": "somepath/", "name": "entry", "type": "file", - "annotations": [] + "annotations": [], } rendered = render_entry(entry) self.assertIn("view/somepath", rendered) @@ -40,7 +40,7 @@ class TestRenderEntry(unittest.TestCase): "path": "somepath", "name": "entry", "type": "file", - "annotations": [] + "annotations": [], } rendered = render_entry(entry) self.assertIn("view/somepath", rendered)