diff --git a/server/app/app.py b/server/app/app.py index 36d4a7cf..780ad6a1 100644 --- a/server/app/app.py +++ b/server/app/app.py @@ -124,7 +124,7 @@ def dataroot_test_index(): data += "

Welcome to cellxgene

" 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) diff --git a/server/cli/launch.py b/server/cli/launch.py index 670d6793..890ebad1 100644 --- a/server/cli/launch.py +++ b/server/cli/launch.py @@ -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) diff --git a/server/common/app_config.py b/server/common/app_config.py index 8247637e..acaaeb8e 100644 --- a/server/common/app_config.py +++ b/server/common/app_config.py @@ -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)] diff --git a/server/test/test_app_config.py b/server/test/test_app_config.py new file mode 100644 index 00000000..689beca9 --- /dev/null +++ b/server/test/test_app_config.py @@ -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)]),