refactor config to support different config options for datasets in different dataroots. (#1596)

This will give us the ability to specify different config options for
different dataroots.

the key of the dataroot dictionary is no longer the same as the dataroot_url.
Previously key==dataroot_url, and now those are separated.

Added an "is_multi_dataset" function to simplify logic where it branched on single vs multi.

Simplified the rest.py interface by no longer passing in the user annotations object, since
that can be retrieved from the dataset.

server/test/__init__.py

@@ -27,21 +27,18 @@ def data_with_tmp_annotations(ext: MatrixDataType, annotations_fixture=False):
     annotations_file = path.join(tmp_dir, "test_annotations.csv")
     if annotations_fixture:
         shutil.copyfile(f"{PROJECT_ROOT}/server/test/test_datasets/pbmc3k-annotations.csv", annotations_file)
-    args = {
-        "embeddings__names": ["umap"],
-        "presentation__max_categories": 100,
-        "single_dataset__obs_names": None,
-        "single_dataset__var_names": None,
-        "diffexp__lfc_cutoff": 0.01,
-    }
     fname = {
         MatrixDataType.H5AD: f"{PROJECT_ROOT}/example-dataset/pbmc3k.h5ad",
         MatrixDataType.CXG: "test/test_datasets/pbmc3k.cxg",
     }[ext]
     data_locator = DataLocator(fname)
     config = AppConfig()
-    config.update(**args)
-    config.update(single_dataset__datapath=data_locator.path)
+    config.update_server_config(
+        single_dataset__obs_names=None, single_dataset__var_names=None, single_dataset__datapath=data_locator.path
+    )
+    config.update_default_dataset_config(
+        embeddings__names=["umap"], presentation__max_categories=100, diffexp__lfc_cutoff=0.01,
+    )
     config.complete_config()
     data = MatrixDataLoader(data_locator.abspath()).open(config)
     annotations = AnnotationsLocalFile(None, annotations_file)
@@ -66,21 +63,21 @@ def skip_if(condition, reason: str):
     return decorator
 
 
-def app_config(data_locator, backed=False, extra={}):
-    args = {
-        "embeddings__names": ["umap", "tsne", "pca"],
-        "presentation__max_categories": 100,
-        "single_dataset__obs_names": None,
-        "single_dataset__var_names": None,
-        "diffexp__lfc_cutoff": 0.01,
-        "adaptor__anndata_adaptor__backed": backed,
-        "single_dataset__datapath": data_locator,
-        "limits__diffexp_cellcount_max": None,
-        "limits__column_request_max": None,
-    }
+def app_config(data_locator, backed=False, extra_server_config={}, extra_dataset_config={}):
     config = AppConfig()
-    config.update(**args)
-    config.update(**extra)
+    config.update_server_config(
+        single_dataset__obs_names=None,
+        single_dataset__var_names=None,
+        adaptor__anndata_adaptor__backed=backed,
+        single_dataset__datapath=data_locator,
+        limits__diffexp_cellcount_max=None,
+        limits__column_request_max=None,
+    )
+    config.update_default_dataset_config(
+        embeddings__names=["umap", "tsne", "pca"], presentation__max_categories=100, diffexp__lfc_cutoff=0.01
+    )
+    config.update_server_config(**extra_server_config)
+    config.update_default_dataset_config(**extra_dataset_config)
     config.complete_config()
     return config
 

server/test/run_diffexp.py

@@ -32,8 +32,8 @@ def main():
     args = parser.parse_args()
 
     app_config = AppConfig()
-    app_config.single_dataset__datapath = args.dataset
-    app_config.server__verbose = True
+    app_config.update_server_config(single_dataset__datapath=args.dataset)
+    app_config.update_server_config(app__verbose=True)
     app_config.complete_config()
 
     loader = MatrixDataLoader(args.dataset)

server/test/test_anndata_adaptor.py

@@ -107,9 +107,9 @@ class AdaptorTest(unittest.TestCase):
             self.assertEqual(len(feature), 1)
 
         check_feature("POST", "/cluster/", False)
-        check_feature("POST", "/diffexp/", self.data.config.diffexp__enable)
+        check_feature("POST", "/diffexp/", self.data.dataset_config.diffexp__enable)
         check_feature("GET", "/layout/obs", True)
-        check_feature("PUT", "/layout/obs", self.data.config.embeddings__enable_reembedding)
+        check_feature("PUT", "/layout/obs", self.data.dataset_config.embeddings__enable_reembedding)
         check_feature("PUT", "/annotations/obs", False)
 
     def test_layout(self):

server/test/test_anndata_adaptor_data_load.py

@@ -15,7 +15,7 @@ class DataLoadAdaptorTest(unittest.TestCase):
     def setUp(self):
         self.data_file = DataLocator(f"{PROJECT_ROOT}/example-dataset/pbmc3k.h5ad")
         config = AppConfig()
-        config.update(single_dataset__datapath=self.data_file.path)
+        config.update_server_config(single_dataset__datapath=self.data_file.path)
         config.complete_config()
         self.data = AnndataAdaptor(self.data_file, config)
 
@@ -40,14 +40,18 @@ class DataLocatorAdaptorTest(unittest.TestCase):
     Test various types of data locators we expect to consume
     """
 
-    def setUp(self):
-        self.args = {
-            "embeddings__names": ["umap"],
-            "presentation__max_categories": 100,
-            "single_dataset__obs_names": None,
-            "single_dataset__var_names": None,
-            "diffexp__lfc_cutoff": 0.01,
-        }
+    def get_basic_config(self):
+        config = AppConfig()
+        config.update_server_config(
+            single_dataset__obs_names=None,
+            single_dataset__var_names=None,
+        )
+        config.update_default_dataset_config(
+            embeddings__names=["umap"],
+            presentation__max_categories=100,
+            diffexp__lfc_cutoff=0.01,
+        )
+        return config
 
     def stdAsserts(self, data):
         """ run these each time we load the data """
@@ -57,9 +61,8 @@ class DataLocatorAdaptorTest(unittest.TestCase):
 
     def test_posix_file(self):
         locator = DataLocator("../example-dataset/pbmc3k.h5ad")
-        config = AppConfig()
-        config.update(**self.args)
-        config.update(single_dataset__datapath=locator.path)
+        config = self.get_basic_config()
+        config.update_server_config(single_dataset__datapath=locator.path)
         config.complete_config()
         data = AnndataAdaptor(locator, config)
         self.stdAsserts(data)
@@ -67,15 +70,13 @@ class DataLocatorAdaptorTest(unittest.TestCase):
     def test_url_https(self):
         url = "https://raw.githubusercontent.com/chanzuckerberg/cellxgene/main/example-dataset/pbmc3k.h5ad"
         locator = DataLocator(url)
-        config = AppConfig()
-        config.update(**self.args)
+        config = self.get_basic_config()
         data = AnndataAdaptor(locator, config)
         self.stdAsserts(data)
 
     def test_url_http(self):
         url = "http://raw.githubusercontent.com/chanzuckerberg/cellxgene/main/example-dataset/pbmc3k.h5ad"
         locator = DataLocator(url)
-        config = AppConfig()
-        config.update(**self.args)
+        config = self.get_basic_config()
         data = AnndataAdaptor(locator, config)
         self.stdAsserts(data)

server/test/test_app_config.py

@@ -11,60 +11,90 @@ import requests
 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)])
+        c.update_server_config(app__verbose=True, multi_dataset__dataroot="datadir")
+        v = c.server_config.changes_from_default()
+        self.assertCountEqual(v, [("app__verbose", True, False), ("multi_dataset__dataroot", "datadir", None)])
 
         c = AppConfig()
-        c.update(server__scripts=(), server__inline_scripts=())
-        v = c.changes_from_default()
+        c.update_default_dataset_config(app__scripts=(), app__inline_scripts=())
+        v = c.server_config.changes_from_default()
         self.assertCountEqual(v, [])
 
         c = AppConfig()
-        c.update(server__scripts=[], server__inline_scripts=[])
-        v = c.changes_from_default()
+        c.update_default_dataset_config(app__scripts=[], app__inline_scripts=[])
+        v = c.default_dataset_config.changes_from_default()
         self.assertCountEqual(v, [])
 
         c = AppConfig()
-        c.update(server__scripts=("a", "b"), server__inline_scripts=["c", "d"])
-        v = c.changes_from_default()
-        self.assertCountEqual(v, [("server__scripts", ["a", "b"], []), ("server__inline_scripts", ["c", "d"], [])])
+        c.update_default_dataset_config(app__scripts=("a", "b"), app__inline_scripts=["c", "d"])
+        v = c.default_dataset_config.changes_from_default()
+        self.assertCountEqual(v, [("app__scripts", ["a", "b"], []), ("app__inline_scripts", ["c", "d"], [])])
 
     def test_multi_dataset(self):
 
         c = AppConfig()
         # test for illegal url_dataroots
-        for illegal in ("a/b", "../b", "!$*", "\\n", "", "(bad)"):
-            c.update(multi_dataset__dataroot={illegal: f"{PROJECT_ROOT}/example-dataset"})
+        for illegal in ("../b", "!$*", "\\n", "", "(bad)"):
+            c.update_server_config(
+                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": "{PROJECT_ROOT}/example-dataset"}}
+            )
             with self.assertRaises(ConfigurationError):
                 c.complete_config()
 
         # test for legal url_dataroots
-        for legal in (
-            "d",
-            "this.is-okay_",
-        ):
-            c.update(multi_dataset__dataroot={legal: f"{PROJECT_ROOT}/example-dataset"})
+        for legal in ("d", "this.is-okay_", "a/b"):
+            c.update_server_config(
+                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": "{PROJECT_ROOT}/example-dataset"}}
+            )
             c.complete_config()
 
         # test that multi dataroots work end to end
-        c.update(
+        c.update_server_config(
             multi_dataset__dataroot=dict(
-                set1=f"{PROJECT_ROOT}/example-dataset", set2=f"{PROJECT_ROOT}/server/test/test_datasets"
+                s1=dict(dataroot=f"{PROJECT_ROOT}/example-dataset", base_url="set1/1/2"),
+                s2=dict(dataroot=f"{PROJECT_ROOT}/server/test/test_datasets", base_url="set2"),
+                s3=dict(dataroot=f"{PROJECT_ROOT}/server/test/test_datasets", base_url="set3"),
             )
         )
+
+        # Change this default to test if the dataroot overrides below work.
+        c.update_default_dataset_config(app__about_legal_tos="tos_default.html")
+
+        # specialize the configs for set1
+        c.add_dataroot_config(
+            "s1", user_annotations__enable=False, diffexp__enable=True, app__about_legal_tos="tos_set1.html"
+        )
+
+        # specialize the configs for set2
+        c.add_dataroot_config(
+            "s2", user_annotations__enable=True, diffexp__enable=False, app__about_legal_tos="tos_set2.html"
+        )
+
+        # no specializations for set3 (they get the default dataset config)
         c.complete_config()
 
         with test_server(app_config=c) as server:
             session = requests.Session()
 
-            r = session.get(f"{server}/set1/pbmc3k.h5ad/api/v0.2/config")
+            r = session.get(f"{server}/set1/1/2/pbmc3k.h5ad/api/v0.2/config")
             data_config = r.json()
             assert data_config["config"]["displayNames"]["dataset"] == "pbmc3k"
+            assert data_config["config"]["parameters"]["annotations"] is False
+            assert data_config["config"]["parameters"]["disable-diffexp"] is False
+            assert data_config["config"]["parameters"]["about_legal_tos"] == "tos_set1.html"
 
             r = session.get(f"{server}/set2/pbmc3k.cxg/api/v0.2/config")
             data_config = r.json()
             assert data_config["config"]["displayNames"]["dataset"] == "pbmc3k"
+            assert data_config["config"]["parameters"]["annotations"] is True
+            assert data_config["config"]["parameters"]["about_legal_tos"] == "tos_set2.html"
+
+            r = session.get(f"{server}/set3/pbmc3k.cxg/api/v0.2/config")
+            data_config = r.json()
+            assert data_config["config"]["displayNames"]["dataset"] == "pbmc3k"
+            assert data_config["config"]["parameters"]["annotations"] is True
+            assert data_config["config"]["parameters"]["disable-diffexp"] is False
+            assert data_config["config"]["parameters"]["about_legal_tos"] == "tos_default.html"
 
             r = session.get(f"{server}/health")
             assert r.json()["status"] == "pass"

server/test/test_diffexp.py

@@ -15,8 +15,9 @@ class DiffExpTest(unittest.TestCase):
     """Tests the diffexp returns the expected results for one test case, using different
     adaptor types and different algorithms."""
 
-    def load_dataset(self, path, extra={}):
-        config = app_config(path, extra=extra)
+    def load_dataset(self, path, extra_server_config={}, extra_dataset_config={}):
+        config = app_config(path, extra_server_config=extra_server_config,
+                            extra_dataset_config=extra_dataset_config)
         loader = MatrixDataLoader(path)
         adaptor = loader.open(config)
         return adaptor
@@ -100,7 +101,7 @@ class DiffExpTest(unittest.TestCase):
             # create a sparse matrix
             h5adfile = os.path.join(dirname, "sparse.h5ad")
             create_test_h5ad(h5adfile, 2000, 2000, 10, apply_col_shift)
-            adaptor_anndata = self.load_dataset(h5adfile, extra=dict(embeddings__names=[]))
+            adaptor_anndata = self.load_dataset(h5adfile, extra_dataset_config=dict(embeddings__names=[]))
             adata = adaptor_anndata.data
 
             sparsename = os.path.join(dirname, "sparse.cxg")

server/test/test_eb.py

@@ -0,0 +1,54 @@
+import unittest
+import tempfile
+import requests
+import subprocess
+from server.test import PROJECT_ROOT
+from server.common.app_config import AppConfig
+from contextlib import contextmanager
+import time
+
+
+@contextmanager
+def run_eb_app(tempdirname):
+    ps = subprocess.Popen(["python", "artifact.dir/application.py"], cwd=tempdirname)
+    server = "http://localhost:5000"
+    for _ in range(10):
+        try:
+            requests.get(f"{server}/health")
+            break
+        except requests.exceptions.ConnectionError:
+            time.sleep(1)
+
+    try:
+        yield server
+    finally:
+        try:
+            ps.terminate()
+        except ProcessLookupError:
+            pass
+
+
+class Elastic_Beanstalk_Test(unittest.TestCase):
+    def test_run(self):
+
+        tempdir = tempfile.TemporaryDirectory(dir=f"{PROJECT_ROOT}/server")
+        tempdirname = tempdir.name
+
+        c = AppConfig()
+        # test that eb works
+        c.update_server_config(
+            multi_dataset__dataroot=f"{PROJECT_ROOT}/server/test/test_datasets", app__flask_secret_key="open sesame"
+        )
+
+        c.complete_config()
+        c.write_config(f"{tempdirname}/config.yaml")
+
+        subprocess.check_call(f"git ls-files . | cpio -pdm {tempdirname}", cwd=f"{PROJECT_ROOT}/server/eb", shell=True)
+        subprocess.check_call(["make", "build"], cwd=tempdirname)
+
+        with run_eb_app(tempdirname) as server:
+            session = requests.Session()
+
+            r = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config")
+            data_config = r.json()
+            assert data_config["config"]["displayNames"]["dataset"] == "pbmc3k"

server/test/test_matrixcache.py

@@ -21,13 +21,13 @@ class MatrixCacheTest(unittest.TestCase):
             shutil.copytree(source, target)
 
     def use_dataset(self, matrix_cache, dirname, app_config, dataset_index):
-        with matrix_cache.data_adaptor(os.path.join(dirname, str(dataset_index) + ".cxg"), app_config) as adaptor:
+        with matrix_cache.data_adaptor(None, os.path.join(dirname, str(dataset_index) + ".cxg"), app_config) as adaptor:
             pass
         return adaptor
 
     def use_dataset_with_error(self, matrix_cache, dirname, app_config, dataset_index):
         try:
-            with matrix_cache.data_adaptor(os.path.join(dirname, str(dataset_index) + ".cxg"), app_config):
+            with matrix_cache.data_adaptor(None, os.path.join(dirname, str(dataset_index) + ".cxg"), app_config):
                 raise DatasetAccessError("something bad happened")
         except DatasetAccessError:
             # the MatrixDataCacheManager rethrows the exception, so catch and ignore
@@ -38,7 +38,7 @@ class MatrixCacheTest(unittest.TestCase):
         result = {}
         for k, v in datasets.items():
             # filter out the dirname and the .cxg from the name
-            newk = int(k[len(dirname) + 1 : -4])
+            newk = int(k[1][len(dirname) + 1 : -4])
             result[newk] = v
 
         return result

server/test/test_writable_annotation.py

@@ -15,12 +15,13 @@ from server.data_common.matrix_loader import MatrixDataType
 class WritableAnnotationTest(unittest.TestCase):
     def setUp(self):
         self.data, self.tmp_dir, self.annotations = data_with_tmp_annotations(MatrixDataType.H5AD)
+        self.data.dataset_config.user_annotations = self.annotations
 
     def tearDown(self):
         shutil.rmtree(self.tmp_dir)
 
     def annotation_put_fbs(self, fbs):
-        annotations_put_fbs_helper(self.data, self.annotations, fbs)
+        annotations_put_fbs_helper(self.data, fbs)
         res = json.dumps({"status": "OK"})
         return res
 
@@ -112,7 +113,7 @@ class WritableAnnotationTest(unittest.TestCase):
         # get
         labels = self.annotations.read_labels(None)
         fbsAll = self.data.annotation_to_fbs_matrix("obs", None, labels)
-        schema = schema_get_helper(self.data, self.annotations)
+        schema = schema_get_helper(self.data)
         annotations = decode_fbs.decode_matrix_FBS(fbsAll)
         obs_index_col_name = schema["annotations"]["obs"]["index"]
         self.assertEqual(annotations["n_rows"], n_rows)
@@ -149,7 +150,7 @@ class WritableAnnotationTest(unittest.TestCase):
             self.assertEqual(len(feature), 1)
 
         check_feature("POST", "/cluster/", False)
-        check_feature("POST", "/diffexp/", self.data.config.diffexp__enable)
+        check_feature("POST", "/diffexp/", self.data.dataset_config.diffexp__enable)
         check_feature("GET", "/layout/obs", True)
-        check_feature("PUT", "/layout/obs", self.data.config.embeddings__enable_reembedding)
+        check_feature("PUT", "/layout/obs", self.data.dataset_config.embeddings__enable_reembedding)
         check_feature("PUT", "/annotations/obs", True)