fork/cellxgene
1
0
Fork 0
mirror of https://github.com/chanzuckerberg/cellxgene.git synced 2026-09-25 18:18:12 +08:00
Code Issues Packages Projects Releases Wiki Activity

Update for app config (#1369)

Browse Source 
fixes an issue with "cellxgene launch" which had a bad interaction between
command line parameters and config file parameters.
Now, the config files are applied first, followed by the parameters that
were provided in the command line.

There is also now a check that each of the config attributes is type checked.
This commit is contained in:
bmccandless
2020-04-08 15:44:56 -07:00
committed by GitHub
parent 1f94c71c22
commit d457988810
4 changed files with 85 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/app/app.py
+1 -1
View File
@@ -124,7 +124,7 @@ def dataroot_test_index():
data += "<body><H1>Welcome to cellxgene</H1>"
config = current_app.app_config
locator = DataLocator(config.multi_dataset__dataroot, config=config)
locator = DataLocator(config.multi_dataset__dataroot, app_config=config)
datasets = []
for fname in locator.ls():
location = path_join(config.multi_dataset__dataroot, fname)
server/cli/launch.py
+12 -4
View File
@@ -358,7 +358,10 @@ def launch(
if config_file:
app_config.update_from_config_file(config_file)
app_config.update(
# Determine which config options were give on the command line.
# Those will override the ones provided in the config file (if provided).
cli_config = AppConfig()
cli_config.update(
server__verbose=verbose,
server__debug=debug,
server__host=host,
@@ -383,6 +386,11 @@ def launch(
diffexp__lfc_cutoff=diffexp_lfc_cutoff,
adaptor__anndata_adaptor__backed=backed,
)
diff = cli_config.changes_from_default()
changes = {}
for key, val, defval in diff:
changes[key] = val
app_config.update(**changes)
# process the configuration
# any errors will be thrown as an exception.
@@ -391,11 +399,11 @@ def launch(
def messagefn(message):
click.echo("[cellxgene] " + message)
app_config.complete_config(messagefn)
# Use a default secret if one is not provided
if not app_config.server__flask_secret_key:
app_config.server__flask_secret_key = "SparkleAndShine"
app_config.update(server__flask_secret_key="SparkleAndShine")
app_config.complete_config(messagefn)
except (ConfigurationError, DatasetAccessError) as e:
raise click.ClickException(e)
server/common/app_config.py
+58 -13
View File
@@ -8,6 +8,7 @@ from os.path import splitext, basename, isdir
import sys
from urllib.parse import urlparse
import yaml
import copy
from server.common.default_config import get_default_config
from server.common.errors import ConfigurationError, DatasetAccessError, OntologyLoadFailure
@@ -53,6 +54,7 @@ class AppConfig(object):
def __init__(self):
self.default_config = get_default_config()
self.attr_checked = {k: False for k in self.__mapping(self.default_config).keys()}
dc = self.default_config
try:
@@ -116,21 +118,40 @@ class AppConfig(object):
# Set to true when config_completed is called
self.is_completed = False
def check_config(self):
if not self.is_completed:
raise ConfigurationError("The configuration has not been completed")
mapping = self.__mapping(self.default_config)
for key in mapping.keys():
if not self.attr_checked[key]:
raise ConfigurationError(f"The attr '{key}' has not been checked")
def __mapping(self, config):
"""Create a mapping from attribute names to (location in the config tree, value)"""
dc = copy.deepcopy(config)
mapping = {}
# special case for tiledb_ctx whose value is a dict.
val = config.get("adaptor", {}).get("cxg_adaptor", {}).get("tiledb_ctx")
if val is not None:
mapping["adaptor__cxg_adaptor__tiledb_ctx"] = (("adaptor", "cxg_adaptor", "tiledb_ctx"), val)
del dc["adaptor"]["cxg_adaptor"]["tiledb_ctx"]
flat_config = flatten(dc)
for key, value in flat_config.items():
# name of the attribute
attr = "__".join(key)
mapping[attr] = (key, value)
return mapping
def update_from_config_file(self, config_file):
with open(config_file) as fyaml:
config = yaml.load(fyaml, Loader=yaml.FullLoader)
# special case for tiledb_ctx whose value is a dict, and cannot
# be handled by the flattening below
if config.get("adaptor", {}).get("cxg_adaptor", {}).get("tiledb_ctx"):
value = config["adaptor"]["cxg_adaptor"]["tiledb_ctx"]
self.adaptor__cxg_adaptor__tiledb_ctx = value
del config["adaptor"]["cxg_adaptor"]["tiledb_ctx"]
flat_config = flatten(config)
for key, value in flat_config.items():
# name of the attribute
attr = "__".join(key)
mapping = self.__mapping(config)
for attr, (key, value) in mapping.items():
if not hasattr(self, attr):
raise ConfigurationError(f"Unknown key from config file: {key}")
try:
@@ -138,6 +159,8 @@ class AppConfig(object):
except KeyError:
raise ConfigurationError(f"Unable to set config attribute: {key}")
self.attr_checked[attr] = False
self.is_completed = False
def update(self, **kw):
@@ -149,8 +172,20 @@ class AppConfig(object):
except KeyError:
raise ConfigurationError(f"Unable to set config parameter {key}.")
self.attr_checked[key] = False
self.is_completed = False
def changes_from_default(self):
"""Return all the attribute that are different from the default"""
mapping = self.__mapping(self.default_config)
diff = []
for attrname, (key, defval) in mapping.items():
curval = getattr(self, attrname)
if curval != defval:
diff.append((attrname, curval, defval))
return diff
def complete_config(self, messagefn=None):
"""The configure options are checked, and any additional setup based on the config
parameters is done"""
@@ -168,6 +203,8 @@ class AppConfig(object):
context = dict(messagefn=messagefn)
self.handle_server(context)
self.handle_data_locator(context)
self.handle_presentation(context)
self.handle_single_dataset(context)
self.handle_multi_dataset(context)
self.handle_user_annotations(context)
@@ -176,6 +213,7 @@ class AppConfig(object):
self.handle_adaptor(context)
self.is_completed = True
self.check_config()
def __check_attr(self, attrname, vtype):
val = getattr(self, attrname)
@@ -192,6 +230,8 @@ class AppConfig(object):
f"expected type {vtype.__name__}, got {type(val).__name__}"
)
self.attr_checked[attrname] = True
def handle_server(self, context):
self.__check_attr("server__verbose", bool)
self.__check_attr("server__debug", bool)
@@ -202,6 +242,8 @@ class AppConfig(object):
self.__check_attr("server__force_https", bool)
self.__check_attr("server__flask_secret_key", (type(None), str))
self.__check_attr("server__generate_cache_control_headers", bool)
self.__check_attr("server__about_legal_tos", (type(None), str))
self.__check_attr("server__about_legal_privacy", (type(None), str))
self.__check_attr("server__server_timing_headers", bool)
if self.server__port:
@@ -227,6 +269,9 @@ class AppConfig(object):
# second, from config file
self.server__flask_secret_key = environ.get("CXG_SECRET_KEY", self.server__flask_secret_key)
def handle_data_locator(self, context):
self.__check_attr("data_locator__s3__region_name", (type(None), str))
def handle_presentation(self, context):
self.__check_attr("presentation__max_categories", int)
@@ -416,8 +461,8 @@ class AppConfig(object):
# FIXME The current set of config is not consistently presented:
# we have camalCase, hyphen-text, and underscore_text
if not self.is_completed:
raise ConfigurationError("The configuration has not been completed")
# make sure the configuration has been checked.
self.check_config()
# features
features = [f.todict() for f in data_adaptor.get_features(annotation)]
server/test/test_app_config.py
+14
View File
@@ -0,0 +1,14 @@
import unittest
from server.common.app_config import AppConfig
# NOTE, there are more tests that should be written for AppConfig.
# this is just a start.
class AppConfigTest(unittest.TestCase):
def test_update(self):
c = AppConfig()
c.update(server__verbose=True, multi_dataset__dataroot="datadir")
v = c.changes_from_default()
self.assertCountEqual(v, [("server__verbose", True, False), ("multi_dataset__dataroot", "datadir", None)]),