From eb108feb37fc4e244b717bb9bc8a1bf14ed55f17 Mon Sep 17 00:00:00 2001
From: Madison Dunitz <madison.dunitz@chanzuckerberg.com>
Date: Wed, 7 Oct 2020 12:36:02 -0500
Subject: [PATCH 01/16] Performance test annotations (#1908)

* make testing plan

* create annotaions sets for different num categories/dataset size

* annotation creation testing

* create scale and perf tests for annotations

* create make commands for tests

* get cell count if not set in test_datasets dict
---
 Makefile                                      |   2 +-
 server/Makefile                               |   8 +
 .../performance_test_annotations_backend.py   | 215 ++++++++++++++++++
 .../performance/scale_test_annotations.py     |  46 ++++
 4 files changed, 270 insertions(+), 1 deletion(-)
 create mode 100644 server/test/performance/performance_test_annotations_backend.py
 create mode 100644 server/test/performance/scale_test_annotations.py

diff --git a/Makefile b/Makefile
index ca8754f7..8f9a4083 100644
--- a/Makefile
+++ b/Makefile
@@ -83,7 +83,7 @@ lint: lint-server lint-client
 
 .PHONY: lint-server
 lint-server: fmt-py
-	flake8 server --per-file-ignores='server/test/fixtures/dataset_config_outline.py:F821 server/test/fixtures/server_config_outline.py:F821'
+	flake8 server --per-file-ignores='server/test/fixtures/dataset_config_outline.py:F821 server/test/fixtures/server_config_outline.py:F821 server/test/performance/scale_test_annotations.py:E501'
 
 
 .PHONY: lint-client
diff --git a/server/Makefile b/server/Makefile
index 700b9a27..016f89cc 100644
--- a/server/Makefile
+++ b/server/Makefile
@@ -39,3 +39,11 @@ create-test-db:
 clean-test-db:
 	-docker stop test_db
 	-docker rm test_db
+
+.PHONY: test-annotations-performance
+test-annotations-performance:
+	python test/performance/performance_test_annotations_backend.py
+
+.PHONY: test-annotations-scale
+test-annotations-scale:
+	locust -f test/performance/scale_test_annotations.py --headless -u 30 -r 10 --host https://api.cellxgene.dev.single-cell.czi.technology/cellxgene/e/ --run-time 5m 2>&1 | tee locust_dev_stats.txt
diff --git a/server/test/performance/performance_test_annotations_backend.py b/server/test/performance/performance_test_annotations_backend.py
new file mode 100644
index 00000000..d0548523
--- /dev/null
+++ b/server/test/performance/performance_test_annotations_backend.py
@@ -0,0 +1,215 @@
+import json
+import string
+from contextlib import contextmanager
+from timeit import default_timer
+import concurrent.futures
+import numpy as np
+import requests
+import sys
+from server.data_common.fbs.matrix import encode_matrix_fbs
+import pandas as pd
+import random
+
+"""
+Before running, sign into the dataportal, copy the cookie and paste it below. To test in staging or prod update the
+url base below. It is also possible to configure the number of categories created and the number of unique labels per
+category.
+"""
+
+cookie = ""
+
+test_datasets = {
+    "smallest": {
+        "dataset_url": "kampmann_lab_human_AD_snRNAseq_EC_inhibitoryNeurons-53-remixed.cxg",
+        "name": "smallest",
+        "num_cells": 5270,
+    },
+    "10k": {
+        "dataset_url": "krasnow_lab_human_lung_cell_atlas_smartseq2-2-remixed.cxg",
+        "name": "10k",
+        "num_cells": 9409,
+    },
+    "80k": {
+        "dataset_url": "Single_cell_gene_expression_profiling_of_SARS_CoV_2_infected_human_cell_lines_H1299-27-remixed.cxg",  # noqa E501
+        "name": "80k",
+        "num_cells": 81736,
+    },
+    "140k": {"dataset_url": "Single_cell_drug_screening_a549-42-remixed.cxg", "name": "140k", "num_cells": 143015},
+    "largest": {"dataset_url": "human_cell_landscape.cxg", "name": "largest", "num_cells": 599926},
+    "1million": {"dataset_url": None, "name": "1million", "num_cells": 1000000},
+    "4million": {"dataset_url": None, "name": "4million", "num_cells": 4000000},
+}
+
+url_base = "https://api.cellxgene.dev.single-cell.czi.technology/cellxgene/e/"
+annotations_category_count = [1, 10, 50]
+max_labels = [5, 50, 100]
+
+
+class PerformanceTestingAnnotations:
+    def __init__(
+        self,
+        datasets=test_datasets,
+        annotations_category_count=annotations_category_count,
+        max_labels=max_labels,
+        url_base=url_base,
+    ):
+        self.test_datasets = datasets
+        self.annotations_category_count = annotations_category_count
+        self.max_labels = max_labels
+        self.url_base = url_base
+        self.test_notes = self.create_info_dict()
+
+    def set_cell_count(self, dataset_name):
+        dataset_url = self.test_datasets[dataset_name]["dataset_url"]
+        headers = {"Content-Type": "application/octet-stream", "Cookie": cookie}
+        response = self.client.get(f"{self.url_base}{dataset_url}/api/v0.2/schema", headers=headers)
+        cell_count = json.loads(response._content)["schema"]["dataframe"]["nObs"]
+        self.test_datasets[dataset_name]["cell_count"] = cell_count
+
+    def create_info_dict(self):
+        request_info = {}
+        for dataset in self.test_datasets.keys():
+            request_info[dataset] = {}
+            for cat_count in self.annotations_category_count:
+                request_info[dataset][f"num_categories_{cat_count}"] = {}
+                for unique_labels in self.max_labels:
+                    request_info[dataset][f"num_categories_{cat_count}"][f"max_label_{unique_labels}"] = {}
+        return request_info
+
+    def create_annotations_dict_multi_process(self, dataset_name, category_count, label_max):
+        annotation_dict = {}
+        futures = []
+        categories = [f"Category{i}" for i in range(category_count)]
+        if not self.test_datasets[dataset_name]["num_cells"]:
+            self.set_cell_count(dataset_name)
+        with concurrent.futures.ProcessPoolExecutor(max_workers=5) as executor:
+            for category in categories:
+                futures.append(
+                    executor.submit(
+                        self.build_array_for_category,
+                        category,
+                        self.test_datasets[dataset_name]["num_cells"],
+                        label_max,
+                    )
+                )
+            for future in concurrent.futures.as_completed(futures):
+                try:
+                    result = future.result()
+                    category_name, cells = result
+                    annotation_dict[category_name] = pd.Series(cells, dtype="category")
+                except Exception as e:
+                    print(f"Issue creating the annotations dict: {e}")
+        return annotation_dict
+
+    def build_array_for_category(self, category_name, cell_count, label_max):
+        unique_label_count = label_max
+        labels = self.generate_labels(unique_label_count)
+        cells_per_label = int(cell_count / len(labels))
+        extra = cell_count % len(labels)
+        cells = []
+        for label in labels:
+            cells.extend([label] * cells_per_label)
+        cells.extend(["extra"] * extra)
+        rng = np.random.default_rng()
+        rng.shuffle(cells)
+        return category_name, cells
+
+    @staticmethod
+    def convert_to_fbs(annotation_dict):
+        df = pd.DataFrame(annotation_dict)
+        return encode_matrix_fbs(matrix=df, row_idx=None, col_idx=df.columns)
+
+    @staticmethod
+    def generate_labels(unique_label_count):
+        labels = ["undefined"]
+        for i in range(unique_label_count):
+            length = random.randrange(10, 20)
+            labels.append(f"{i}__" + "".join(random.choice(string.ascii_letters) for z in range(length)))
+        return labels
+
+    @contextmanager
+    def elapsed_timer(self):
+        start = default_timer()
+        elapser = lambda: default_timer() - start  # noqa E731
+        yield lambda: elapser()
+        end = default_timer()
+        elapser = lambda: end - start  # noqa E731
+
+    def create_matrix(self, dataset_name, num_cat, max_labels):
+        with self.elapsed_timer() as elapsed:
+            annon_dict = self.create_annotations_dict_multi_process(dataset_name, num_cat, max_labels)
+            dict_size = sum(sys.getsizeof(value) for value in annon_dict.values()) / 1024 ** 2
+            self.test_notes[dataset_name][f"num_categories_{num_cat}"][f"max_label_{max_labels}"]["annotation_dict"] = {
+                "creation_time": str(elapsed()),
+                "size": f"{dict_size} mb",
+            }
+            df = pd.DataFrame(annon_dict)
+            df_size = sys.getsizeof(df) / 1024 ** 2
+            self.test_notes[dataset_name][f"num_categories_{num_cat}"][f"max_label_{max_labels}"]["data_frame"] = {
+                "creation_time": str(elapsed()),
+                "size": f"{df_size} mb",
+            }
+            try:
+                matrix = encode_matrix_fbs(matrix=df, row_idx=None, col_idx=df.columns)
+                matrix_size = sys.getsizeof(matrix) / 1024 ** 2
+                self.test_notes[dataset_name][f"num_categories_{num_cat}"][f"max_label_{max_labels}"]["fbs_matrix"] = {
+                    "creation_time": str(elapsed()),
+                    "size": f"{matrix_size} mb",
+                }
+                return matrix
+            except Exception as e:
+                print(f"Issue creating fbs matrix: {e}, for {dataset_name}")
+                return []
+
+    def send_put_request(self, dataset_url, data):
+        url = self.url_base + f"{dataset_url}/api/v0.2/annotations/obs"
+        with self.elapsed_timer() as elapsed:
+            try:
+                headers = {"Content-Type": "application/octet-stream", "Cookie": cookie}
+                response = requests.put(url=url, data=data, headers=headers)
+            except Exception as e:
+                print(f"Issue with put request: {e}")
+                return None, elapsed()
+            return response, elapsed()
+
+    def test_categories_max_label_matrix(self, dataset_name):
+        for unique_labels in self.max_labels:
+            for category_count in self.annotations_category_count:
+                print(f"Starting dataset: {dataset_name}, categories: {category_count}, labels: {unique_labels}")
+                fbs_matrix = self.create_matrix(dataset_name, category_count, unique_labels)
+                if self.test_datasets[dataset_name]["dataset_url"] and fbs_matrix:
+                    response, response_time = self.send_put_request(
+                        self.test_datasets[dataset_name]["dataset_url"], fbs_matrix
+                    )
+                    if response is None:
+                        self.test_notes[dataset_name][f"num_categories_{category_count}"][f"max_label_{unique_labels}"][
+                            "put_request"
+                        ] = {"response_status": "failed", "request_time": str(response_time)}
+                    else:
+                        self.test_notes[dataset_name][f"num_categories_{category_count}"][f"max_label_{unique_labels}"][
+                            "put_request"
+                        ] = {"response_status": response.status_code, "request_time": str(response_time)}
+
+
+def test_all_datasets():
+    """
+    Run time is dependent on number of datasets, dataset size, number of categories/number being tested and number of
+     unique label counts being tested. However it generally takes a long time. I recommend running this in tmux
+    """
+    perf_test = PerformanceTestingAnnotations()
+    for dataset_name in perf_test.test_datasets.keys():
+        print(f"Testing annotation creation for: {dataset_name}")
+        try:
+            perf_test.test_categories_max_label_matrix(dataset_name)
+        except Exception as e:
+            print(f"something went wrong with {dataset_name}: {e}")
+    return perf_test.test_notes
+
+
+def main():
+    notes = test_all_datasets()
+    print(notes)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/server/test/performance/scale_test_annotations.py b/server/test/performance/scale_test_annotations.py
new file mode 100644
index 00000000..5aa2ac6e
--- /dev/null
+++ b/server/test/performance/scale_test_annotations.py
@@ -0,0 +1,46 @@
+import time
+import random
+
+from locust import HttpUser, between, task
+
+random.seed(time.time())
+"""
+To run this script sign into cellxgene in the desired environment and grab the returned cookie, update the cookie
+variable below with your cookie and run the following command to see results in the terminal:
+locust -f server/test/performance/scale_test_annotations.py --headless -u 30 -r 10 --host https://api.cellxgene.dev.single-cell.czi.technology/cellxgene/e/ --run-time 5m 2>&1 | tee locust_dev_stats.txt
+
+Or if you want to use the locust gui run:
+locust -f server/test/performance/scale_test_annotations.py -u 30 -r 10 --host https://api.cellxgene.dev.single-cell.czi.technology/cellxgene/e/
+
+If you want to test staging you'll need to substitute staging for dev in the host url
+To test prod you'll need to replace dev.single-cell.czi.technology with cziscience.com
+If you'd like to test additional datasets you'll need to add them to the dataset_urls array
+
+Todo @mdunitz update script to retrieve different annotation categories -- may need to create them to ensure the
+categories are shared across datasets for a given user.
+"""
+cookie = ""
+
+
+class WebsiteUser(HttpUser):
+    wait_time = between(1, 2)
+    dataset_urls = [
+        "human_cell_landscape.cxg",
+        "Single_cell_drug_screening_a549-42-remixed.cxg",
+        "kampmann_lab_human_AD_snRNAseq_EC_inhibitoryNeurons-53-remixed.cxg",
+        "krasnow_lab_human_lung_cell_atlas_smartseq2-2-remixed.cxg",
+        "Single_cell_gene_expression_profiling_of_SARS_CoV_2_infected_human_cell_lines_H1299-27-remixed.cxg",
+    ]
+
+    @task
+    def get_annotations(self):
+        dataset_url = random.choice(self.dataset_urls)
+        url = f"{dataset_url}/api/v0.2/annotations/obs?annotation-name=cell_type"
+        headers = {"Content-Type": "application/octet-stream", "Cookie": cookie}
+        self.client.get(url, headers=headers)
+
+    @task
+    def get_schema(self):
+        dataset_url = random.choice(self.dataset_urls)
+        headers = {"Content-Type": "application/octet-stream", "Cookie": cookie}
+        self.client.get(f"{dataset_url}/api/v0.2/schema", headers=headers)

From cf77a8da9e4138af1e95a41071227140df064e4b Mon Sep 17 00:00:00 2001
From: bmccandless <bmccandless@chanzuckerberg.com>
Date: Wed, 7 Oct 2020 12:17:23 -0700
Subject: [PATCH 02/16] Add "picture" to the /userinfo endpoint. (#1914)

* Add "picture" to the /userinfo endpoint.

This may be null or a URL.
 add picture for the test authentication method
---
 server/auth/auth.py                   |  4 ++++
 server/auth/auth_oauth.py             | 16 +++++++---------
 server/auth/auth_test.py              |  6 ++++++
 server/common/config/client_config.py |  1 +
 server/test/unit/auth/test_auth.py    |  7 +++++++
 5 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/server/auth/auth.py b/server/auth/auth.py
index 145616cc..03184e8d 100644
--- a/server/auth/auth.py
+++ b/server/auth/auth.py
@@ -43,6 +43,10 @@ class AuthTypeBase(ABC):
         """Return the name of the user (string)"""
         pass
 
+    def get_user_picture(self):
+        """Return the location to the user's picture"""
+        return None
+
 
 class AuthTypeClientBase(AuthTypeBase):
     """Base type for all authentication types that require the client to login"""
diff --git a/server/auth/auth_oauth.py b/server/auth/auth_oauth.py
index d7162318..b2724c57 100644
--- a/server/auth/auth_oauth.py
+++ b/server/auth/auth_oauth.py
@@ -146,21 +146,19 @@ class AuthTypeOAuth(AuthTypeClientBase):
 
     def get_user_id(self):
         payload = self.get_userinfo()
-        if payload and payload.get("sub"):
-            return payload.get("sub")
-        return None
+        return payload.get("sub") if payload else None
 
     def get_user_name(self):
         payload = self.get_userinfo()
-        if payload and payload.get("name"):
-            return payload.get("name")
-        return None
+        return payload.get("name") if payload else None
 
     def get_user_email(self):
         payload = self.get_userinfo()
-        if payload and payload.get("email"):
-            return payload.get("email")
-        return None
+        return payload.get("email") if payload else None
+
+    def get_user_picture(self):
+        payload = self.get_userinfo()
+        return payload.get("picture") if payload else None
 
     def update_response(self, response):
         response.cache_control.update(dict(public=True, max_age=0, no_store=True, no_cache=True, must_revalidate=True))
diff --git a/server/auth/auth_test.py b/server/auth/auth_test.py
index 4594dae0..06b92cc7 100644
--- a/server/auth/auth_test.py
+++ b/server/auth/auth_test.py
@@ -10,12 +10,14 @@ class AuthTypeTest(AuthTypeClientBase):
     CXGUID = "cxguid_test"
     CXGUNAME = "cxguname_test"
     CXGUEMAIL = "cxguemail_test"
+    CXGUPICTURE = "cxgupicture_test"
 
     def __init__(self, app_config):
         super().__init__()
         self.user_name = "test_account"
         self.user_id = "id0001"
         self.user_email = "test_account@test.com"
+        self.user_picture = None
 
     def is_valid_authentication_type(self):
         return True
@@ -42,12 +44,16 @@ class AuthTypeTest(AuthTypeClientBase):
     def get_user_email(self):
         return session.get(self.CXGUEMAIL)
 
+    def get_user_picture(self):
+        return session.get(self.CXGUPICTURE)
+
     def login(self):
         args = request.args
         return_to = args.get("dataset", "/")
         session[self.CXGUID] = args.get("userid", self.user_id)
         session[self.CXGUNAME] = args.get("username", self.user_name)
         session[self.CXGUEMAIL] = args.get("email", self.user_email)
+        session[self.CXGUPICTURE] = args.get("picture", self.user_picture)
         return redirect(return_to)
 
     def logout(self):
diff --git a/server/common/config/client_config.py b/server/common/config/client_config.py
index ccddf2c1..8a70c0b6 100644
--- a/server/common/config/client_config.py
+++ b/server/common/config/client_config.py
@@ -117,5 +117,6 @@ def get_client_userinfo(app_config, data_adaptor):
             "username": auth.get_user_name(),
             "user_id": auth.get_user_id(),
             "email": auth.get_user_email(),
+            "picture": auth.get_user_picture(),
         }
         return userinfo
diff --git a/server/test/unit/auth/test_auth.py b/server/test/unit/auth/test_auth.py
index f7f6f358..0b8c0bf1 100644
--- a/server/test/unit/auth/test_auth.py
+++ b/server/test/unit/auth/test_auth.py
@@ -82,6 +82,7 @@ class AuthTest(unittest.TestCase):
             userinfo = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/userinfo").json()
             self.assertTrue(userinfo["userinfo"]["is_authenticated"])
             self.assertEqual(userinfo["userinfo"]["username"], "test_account")
+            self.assertEqual(userinfo["userinfo"]["picture"], None)
             self.assertTrue(config["config"]["parameters"]["annotations"])
 
             r = session.get(f"{server}/{logout_uri}")
@@ -100,6 +101,12 @@ class AuthTest(unittest.TestCase):
             self.assertIsNone(userinfo)
             self.assertFalse(config["config"]["parameters"]["annotations"])
 
+            # login with a picture
+            r = session.get(f"{server}/{login_uri}&picture=myimage.png")
+            userinfo = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/userinfo").json()
+            self.assertTrue(userinfo["userinfo"]["is_authenticated"])
+            self.assertEqual(userinfo["userinfo"]["picture"], "myimage.png")
+
     def test_auth_test_single(self):
         c = AppConfig()
         c.update_server_config(

From 1c4c501c43b24504b26b289c3668b5d544dd8292 Mon Sep 17 00:00:00 2001
From: Severiano Badajoz <sbadajoz@chanzuckerberg.com>
Date: Wed, 7 Oct 2020 15:02:55 -0700
Subject: [PATCH 03/16] Auth UI tweaks (#1915)

* remove auth buttons and dataset info from info menu

* add auth buttons to menubar

* remove auth from top left

* new auth buttons

* move infomenu to lsb dir

* styling fixes

* feedback

* more feedback

Co-authored-by: Timmy Huang <thuang@chanzuckerberg.com>
---
 client/src/components/leftSidebar/infoMenu.js |  72 ++++++++++++
 .../leftSidebar/topLeftLogoAndTitle.js        |  14 +--
 client/src/components/menubar/authButtons.js  |  87 +++++++++++---
 client/src/components/menubar/index.js        |   4 +
 client/src/components/menubar/infoMenu.js     | 108 ------------------
 5 files changed, 148 insertions(+), 137 deletions(-)
 create mode 100644 client/src/components/leftSidebar/infoMenu.js
 delete mode 100644 client/src/components/menubar/infoMenu.js

diff --git a/client/src/components/leftSidebar/infoMenu.js b/client/src/components/leftSidebar/infoMenu.js
new file mode 100644
index 00000000..6ab9bdde
--- /dev/null
+++ b/client/src/components/leftSidebar/infoMenu.js
@@ -0,0 +1,72 @@
+// jshint esversion: 6
+import React from "react";
+import { Button, Menu, MenuItem, Popover, Position } from "@blueprintjs/core";
+import { IconNames } from "@blueprintjs/icons";
+
+const InformationMenu = React.memo((props) => {
+  const { libraryVersions, tosURL, privacyURL } = props;
+  return (
+    <Popover
+      content={
+        <Menu>
+          <MenuItem
+            href="https://chanzuckerberg.github.io/cellxgene/"
+            target="_blank"
+            icon="book"
+            text="Documentation"
+            rel="noopener"
+          />
+          <MenuItem
+            href="https://join-cellxgene-users.herokuapp.com/"
+            target="_blank"
+            icon="chat"
+            text="Chat"
+            rel="noopener"
+          />
+          <MenuItem
+            href="https://github.com/chanzuckerberg/cellxgene"
+            target="_blank"
+            icon="git-branch"
+            text="Github"
+            rel="noopener"
+          />
+          <MenuItem target="_blank" text={libraryVersions?.cellxgene || null} />
+          <MenuItem text="MIT License" />
+          {tosURL && (
+            <MenuItem
+              href={tosURL}
+              target="_blank"
+              text="Terms of Service"
+              rel="noopener"
+            />
+          )}
+          {privacyURL && (
+            <MenuItem
+              href={privacyURL}
+              target="_blank"
+              text="Privacy Policy"
+              rel="noopener"
+            />
+          )}
+        </Menu>
+      }
+      position={Position.BOTTOM_RIGHT}
+      modifiers={{
+        preventOverflow: { enabled: false },
+        hide: { enabled: false },
+      }}
+    >
+      <Button
+        data-testid="menu"
+        type="button"
+        icon={IconNames.INFO_SIGN}
+        style={{
+          cursor: "pointer",
+          verticalAlign: "middle",
+        }}
+      />
+    </Popover>
+  );
+});
+
+export default InformationMenu;
diff --git a/client/src/components/leftSidebar/topLeftLogoAndTitle.js b/client/src/components/leftSidebar/topLeftLogoAndTitle.js
index edfc8cef..e4f3d5a7 100644
--- a/client/src/components/leftSidebar/topLeftLogoAndTitle.js
+++ b/client/src/components/leftSidebar/topLeftLogoAndTitle.js
@@ -7,15 +7,12 @@ import * as globals from "../../globals";
 import Logo from "../framework/logo";
 import Truncate from "../util/truncate";
 import InfoDrawer from "../infoDrawer/infoDrawer";
-import AuthButtons from "../menubar/authButtons";
-import InformationMenu from "../menubar/infoMenu";
+import InformationMenu from "./infoMenu";
 
 const DATASET_TITLE_FONT_SIZE = 14;
 
 @connect((state) => ({
   datasetTitle: state.config?.displayNames?.dataset ?? "",
-  auth: state.config?.authentication,
-  userinfo: state.userinfo,
   libraryVersions: state.config?.["library_versions"],
   aboutLink: state.config?.links?.["about-dataset"],
   tosURL: state.config?.parameters?.["about_legal_tos"],
@@ -30,8 +27,6 @@ class LeftSideBar extends React.Component {
   render() {
     const {
       datasetTitle,
-      auth,
-      userinfo,
       libraryVersions,
       aboutLink,
       privacyURL,
@@ -79,7 +74,7 @@ class LeftSideBar extends React.Component {
             gene
           </span>
         </div>
-        <div style={{ marginRight: 5, position: "relative", top: -7 }}>
+        <div style={{ marginRight: 5, height: "100%" }}>
           <Button
             minimal
             style={{
@@ -102,14 +97,9 @@ class LeftSideBar extends React.Component {
               aboutLink,
               tosURL,
               privacyURL,
-              auth,
               dispatch,
-              userinfo,
             }}
           />
-          {!userinfo.is_authenticated ? (
-            <AuthButtons auth={auth} userinfo={userinfo} />
-          ) : null}
         </div>
       </div>
     );
diff --git a/client/src/components/menubar/authButtons.js b/client/src/components/menubar/authButtons.js
index 985b3da3..60ace82b 100644
--- a/client/src/components/menubar/authButtons.js
+++ b/client/src/components/menubar/authButtons.js
@@ -1,30 +1,83 @@
 import React from "react";
-import { AnchorButton, ButtonGroup, Tooltip } from "@blueprintjs/core";
+import {
+  AnchorButton,
+  Button,
+  MenuItem,
+  Tooltip,
+  Popover,
+  Menu,
+} from "@blueprintjs/core";
+import { IconNames } from "@blueprintjs/icons";
+
 import * as globals from "../../globals";
 import styles from "./menubar.css";
 
+const BASE_EMOJI = [0x1f9d1, 0x1f468, 0x1f469];
+const SKIN_TONES = [0x1f3fb, 0x1f3fc, 0x1f3fd, 0x1f3fe, 0x1f3ff];
+const MICROSCOPE = 0x1f52c;
+const ZERO_WIDTH_JOINER = 0x0200d;
+
 const Auth = React.memo((props) => {
   const { auth, userinfo } = props;
 
-  if (!auth || (auth && !auth.requires_client_login)) return null;
+  const randomInt = Math.random() * 15;
+  const sexIndex = Math.floor(randomInt / 5);
+  const skinToneIndex = Math.floor(randomInt % 5);
+
+  const scientist = String.fromCodePoint(
+    BASE_EMOJI[sexIndex],
+    SKIN_TONES[skinToneIndex],
+    ZERO_WIDTH_JOINER,
+    MICROSCOPE
+  );
+
+  if (!auth?.["requires_client_login"]) return null;
+
+  if (userinfo?.["is_authenticated"]) {
+    const PopoverContent = (
+      <Menu>
+        <MenuItem
+          data-testid="user-email"
+          text={`Logged in as: ${userinfo.email}`}
+        />
+        <MenuItem
+          data-testid="log-out"
+          text="Log Out"
+          href={auth.logout}
+          icon={IconNames.LOG_OUT}
+        />
+      </Menu>
+    );
+
+    return (
+      <Popover content={PopoverContent}>
+        <Button className={styles.menubarButton} style={{ padding: 0 }}>
+          {userinfo?.picture ? (
+            <img alt="profile" src={userinfo?.picture} />
+          ) : (
+            <span style={{ fontSize: "18px" }}>{scientist}</span>
+          )}
+        </Button>
+      </Popover>
+    );
+  }
 
   return (
-    <ButtonGroup className={styles.menubarButton}>
-      <Tooltip
-        content="Log in to cellxgene"
-        position="bottom"
-        hoverOpenDelay={globals.tooltipHoverOpenDelay}
+    <Tooltip
+      content="Log in to cellxgene"
+      position="bottom"
+      hoverOpenDelay={globals.tooltipHoverOpenDelay}
+    >
+      <AnchorButton
+        type="button"
+        data-testid="log-in"
+        disabled={false}
+        href={auth.login}
+        className={styles.menubarButton}
       >
-        <AnchorButton
-          type="button"
-          data-testid="auth-button"
-          disabled={false}
-          href={!userinfo.is_authenticated ? auth.login : auth.logout}
-        >
-          {!userinfo.is_authenticated ? "Log In" : "Log Out"}
-        </AnchorButton>
-      </Tooltip>
-    </ButtonGroup>
+        Log In
+      </AnchorButton>
+    </Tooltip>
   );
 });
 
diff --git a/client/src/components/menubar/index.js b/client/src/components/menubar/index.js
index b4271302..115eb6ff 100644
--- a/client/src/components/menubar/index.js
+++ b/client/src/components/menubar/index.js
@@ -7,6 +7,7 @@ import styles from "./menubar.css";
 import actions from "../../actions";
 import Clip from "./clip";
 
+import AuthButtons from "./authButtons";
 import Subset from "./subset";
 import UndoRedoReset from "./undoRedo";
 import DiffexpButtons from "./diffexpButtons";
@@ -216,6 +217,8 @@ class MenuBar extends React.PureComponent {
       subsetPossible,
       subsetResetPossible,
       enableReembedding,
+      userinfo,
+      auth,
     } = this.props;
     const { pendingClipPercentiles } = this.state;
 
@@ -241,6 +244,7 @@ class MenuBar extends React.PureComponent {
           zIndex: 3,
         }}
       >
+        <AuthButtons auth={auth} userinfo={userinfo} />
         <UndoRedoReset
           dispatch={dispatch}
           undoDisabled={undoDisabled}
diff --git a/client/src/components/menubar/infoMenu.js b/client/src/components/menubar/infoMenu.js
deleted file mode 100644
index 06b6bb60..00000000
--- a/client/src/components/menubar/infoMenu.js
+++ /dev/null
@@ -1,108 +0,0 @@
-// jshint esversion: 6
-import React from "react";
-import {
-  Button,
-  ButtonGroup,
-  Classes,
-  Menu,
-  MenuItem,
-  Popover,
-  Position,
-} from "@blueprintjs/core";
-import styles from "./menubar.css";
-
-const handleClick = (dispatch) => {
-  dispatch({ type: "toggle dataset drawer" });
-};
-
-const InformationMenu = React.memo((props) => {
-  const {
-    libraryVersions,
-    tosURL,
-    privacyURL,
-    auth,
-    userinfo,
-    dispatch,
-  } = props;
-  return (
-    <ButtonGroup className={`${styles.menubarButton}`}>
-      <Popover
-        content={
-          <Menu>
-            <MenuItem
-              onClick={() => handleClick(dispatch)}
-              icon="info-sign"
-              text="Dataset Overview"
-            />
-
-            <MenuItem
-              href="https://chanzuckerberg.github.io/cellxgene/"
-              target="_blank"
-              icon="book"
-              text="Documentation"
-              rel="noopener"
-            />
-            <MenuItem
-              href="https://join-cellxgene-users.herokuapp.com/"
-              target="_blank"
-              icon="chat"
-              text="Chat"
-              rel="noopener"
-            />
-            <MenuItem
-              href="https://github.com/chanzuckerberg/cellxgene"
-              target="_blank"
-              icon="git-branch"
-              text="Github"
-              rel="noopener"
-            />
-            <MenuItem
-              target="_blank"
-              text={
-                libraryVersions && libraryVersions.cellxgene
-                  ? libraryVersions.cellxgene
-                  : null
-              }
-            />
-            <MenuItem text="MIT License" />
-            {tosURL ? (
-              <MenuItem href={tosURL} target="_blank" text="Terms of Service" />
-            ) : null}
-            {privacyURL ? (
-              <MenuItem
-                href={privacyURL}
-                target="_blank"
-                text="Privacy Policy"
-                rel="noopener"
-              />
-            ) : null}
-
-            {auth?.["requires_client_login"] &&
-            userinfo?.["is_authenticated"] ? (
-              <>
-                <MenuItem text={`Logged in as: ${userinfo.email}`} />
-                <MenuItem text="Log Out" href={auth.logout} />
-              </>
-            ) : null}
-          </Menu>
-        }
-        position={Position.BOTTOM_RIGHT}
-        modifiers={{
-          preventOverflow: { enabled: false },
-          hide: { enabled: false },
-        }}
-      >
-        <Button
-          data-testid="menu"
-          type="button"
-          className={`${Classes.BUTTON} bp3-icon-info-sign`}
-          style={{
-            cursor: "pointer",
-          }}
-        />
-      </Popover>
-    </ButtonGroup>
-  );
-});
-
-export default InformationMenu;

From 6c1756f85260f81754921536a849ca14e34a4f0b Mon Sep 17 00:00:00 2001
From: bmccandless <bmccandless@chanzuckerberg.com>
Date: Wed, 7 Oct 2020 15:38:42 -0700
Subject: [PATCH 04/16] Enhance the AppConfig with external config sources.
 (#1904)

* Enhance the AppConfig with external config sources.

The external config sources are currently environment variables
and AWS secrets manager.

The config file can be augmented with a section describing how
environmen variables and secrets can update config parameters.

benefits:
 - it will enable the config to draw from more than one secret.  This is useful
   for shared secrets between cellxgene and data portal, as well as auth0 secrets.
 - it will make it very straightforward to check the config before a deployment.

 Part of #1859
---
 server/common/aws_secret_utils.py             |   2 +-
 server/common/config/app_config.py            |  70 +++++-
 server/common/config/base_config.py           |  19 ++
 server/common/config/dataset_config.py        |  24 +-
 server/common/config/external_config.py       |  96 ++++++++
 server/common/config/server_config.py         |  30 +--
 server/common/utils/type_conversion_utils.py  |  14 ++
 server/default_config.py                      |  55 +++++
 server/test/__init__.py                       |   8 +-
 server/test/unit/common/config/__init__.py    |  51 +++-
 .../unit/common/config/test_app_config.py     |  74 ++++++
 .../unit/common/config/test_dataset_config.py |   4 +-
 .../common/config/test_external_config.py     | 231 ++++++++++++++++++
 .../unit/common/config/test_server_config.py  |   9 +-
 14 files changed, 632 insertions(+), 55 deletions(-)
 create mode 100644 server/common/config/external_config.py
 create mode 100644 server/test/unit/common/config/test_external_config.py

diff --git a/server/common/aws_secret_utils.py b/server/common/aws_secret_utils.py
index 070ed160..ce40794d 100644
--- a/server/common/aws_secret_utils.py
+++ b/server/common/aws_secret_utils.py
@@ -18,6 +18,6 @@ def get_secret_key(region_name, secret_name):
             return secret
     except Exception as e:
         logging.critical(f"Caught exception during get_secret_key, {e}", exc_info=True)
-        raise SecretKeyRetrievalError
+        raise SecretKeyRetrievalError(str(e))
 
     return None
diff --git a/server/common/config/app_config.py b/server/common/config/app_config.py
index 422dcb8e..5cd4c4a5 100644
--- a/server/common/config/app_config.py
+++ b/server/common/config/app_config.py
@@ -4,6 +4,7 @@ from flatten_dict import unflatten
 from server.default_config import get_default_config
 from server.common.config.dataset_config import DatasetConfig
 from server.common.config.server_config import ServerConfig
+from server.common.config.external_config import ExternalConfig
 from server.common.errors import ConfigurationError
 
 
@@ -44,6 +45,9 @@ class AppConfig(object):
         #   dataroot config
         self.dataroot_config = {}
 
+        #  external config
+        self.external_config = ExternalConfig(self, self.default_config["external"])
+
         # Set to true when config_completed is called
         self.is_completed = False
 
@@ -61,6 +65,7 @@ class AppConfig(object):
         self.default_dataset_config.check_config()
         for dataset_config in self.dataroot_config.values():
             dataset_config.check_config()
+        self.external_config.check_config()
 
     def update_server_config(self, **kw):
         self.server_config.update(**kw)
@@ -73,6 +78,51 @@ class AppConfig(object):
             value.update(**kw)
         self.is_complete = False
 
+    def update_single_config_from_path_and_value(self, path, value):
+        """Update a single config parameter with the value.
+        Path is a list of string, that gives a path to the config parameter to be updated.
+        For example, path may be ["server","app","port"].
+        """
+        self.is_complete = False
+        if not isinstance(path, list):
+            raise ConfigurationError(f"path must be a list of strings, got '{str(path)}'")
+        for part in path:
+            if not isinstance(part, str):
+                raise ConfigurationError(f"path must be a list of strings, got '{str(path)}'")
+
+        if len(path) < 1 or path[0] not in ("server", "dataset", "per_dataset_config"):
+            raise ConfigurationError("path must start with 'server', 'dataset', or 'per_dataset_config'")
+
+        if path[0] == "server":
+            attr = "__".join(path[1:])
+            try:
+                self.update_server_config(**{attr: value})
+            except ConfigurationError:
+                raise ConfigurationError(f"unknown config parameter at path: '{str(path)}'")
+        elif path[0] == "dataset":
+            attr = "__".join(path[1:])
+            try:
+                self.update_default_dataset_config(**{attr: value})
+            except ConfigurationError:
+                raise ConfigurationError(f"unknown config parameter at path: '{str(path)}'")
+
+        elif path[0] == "per_dataset_config":
+            if len(path) < 2:
+                raise ConfigurationError(f"missing dataroot when using per_dataset_config: got '{path}'")
+            dataroot = path[1]
+            if dataroot not in self.dataroot_config:
+                dataroots = str(list(self.dataroot_config.keys()))
+                raise ConfigurationError(
+                    f"unknown dataroot when using per_dataset_config: got '{path}',"
+                    f" dataroots specified in config are {dataroots}"
+                )
+
+            attr = "__".join(path[2:])
+            try:
+                self.dataroot_config[dataroot].update(**{attr: value})
+            except ConfigurationError:
+                raise ConfigurationError(f"unknown config parameter at path: '{str(path)}'")
+
     def update_from_config_file(self, config_file):
         try:
             with open(config_file) as yml_file:
@@ -94,12 +144,16 @@ class AppConfig(object):
             # then apply the per dataset configuration
             self.dataroot_config[key].update_from_config(dataroot_config, f"per_dataset_config__{key}")
 
+        if config.get("external"):
+            self.external_config.update_from_config(config["external"], "external")
+
         self.is_complete = False
 
-    def write_config(self, config_file):
-        """output the config to a yaml file"""
+    def config_to_dict(self):
+        """return the configuration as an unflattened dict"""
         server = self.server_config.create_mapping(self.server_config.default_config)
         dataset = self.default_dataset_config.create_mapping(self.default_dataset_config.default_config)
+        external = self.external_config.create_mapping(self.external_config.default_config)
         config = dict(server={}, dataset={})
         for attrname in server.keys():
             config["server__" + attrname] = getattr(self.server_config, attrname)
@@ -111,15 +165,23 @@ class AppConfig(object):
             dataset = dataroot_config.create_mapping(dataroot_config.default_config)
             for attrname in dataset.keys():
                 config[f"per_dataset_config__{dataroot_tag}__" + attrname] = getattr(dataroot_config, attrname)
+        for attrname in external.keys():
+            config["external__" + attrname] = getattr(self.external_config, attrname)
 
         config = unflatten(config, splitter=lambda key: key.split("__"))
+        return config
+
+    def write_config(self, config_file):
+        """output the config to a yaml file"""
+        config = self.config_to_dict()
         yaml.dump(config, open(config_file, "w"))
 
     def changes_from_default(self):
         """Return all the attribute that are different from the default"""
         diff_server = self.server_config.changes_from_default()
         diff_dataset = self.default_dataset_config.changes_from_default()
-        diff = dict(server=diff_server, dataset=diff_dataset)
+        diff_external = self.external.changes_from_default()
+        diff = dict(server=diff_server, dataset=diff_dataset, external=diff_external)
         return diff
 
     def add_dataroot_config(self, dataroot_tag, **kw):
@@ -154,6 +216,8 @@ class AppConfig(object):
         # messages we can give correct context for attributes with bad value.
         context = dict(messagefn=messagefn)
 
+        # complete config for external_config first, since this may update values in the other sections
+        self.external_config.complete_config(context)
         self.server_config.complete_config(context)
         self.default_dataset_config.complete_config(context)
         for dataroot_config in self.dataroot_config.values():
diff --git a/server/common/config/base_config.py b/server/common/config/base_config.py
index a0b46f1b..6b9087e7 100644
--- a/server/common/config/base_config.py
+++ b/server/common/config/base_config.py
@@ -80,8 +80,27 @@ class BaseConfig(object):
                 raise ConfigurationError(f"The attr '{key}' has not been checked")
 
     def update(self, **kw):
+        """Update the attributes defined in kw with their new values."""
         for key, value in kw.items():
             if not hasattr(self, key):
+
+                # check if the key is setting into a dictval entry.
+                found_dictval = False
+                for dictval in self.dictval_cases:
+                    dictvalname = "__".join(dictval)
+                    if dictvalname + "__" in key:
+                        dictkey = key[len(dictvalname) + 2 :]
+                        curdictval = getattr(self, dictvalname)
+                        if curdictval is None:
+                            setattr(self, dictvalname, dict(dictkey=value))
+                        else:
+                            curdictval[dictkey] = value
+
+                        found_dictval = True
+                        break
+
+                if found_dictval:
+                    continue
                 raise ConfigurationError(f"unknown config parameter {key}.")
             try:
                 if type(value) == tuple:
diff --git a/server/common/config/dataset_config.py b/server/common/config/dataset_config.py
index 8c8231ac..7586ec2f 100644
--- a/server/common/config/dataset_config.py
+++ b/server/common/config/dataset_config.py
@@ -30,22 +30,18 @@ class DatasetConfig(BaseConfig):
             self.user_annotations__type = default_config["user_annotations"]["type"]
             self.user_annotations__local_file_csv__directory = default_config["user_annotations"]["local_file_csv"][
                 "directory"
-            ]  # noqa E501
+            ]
             self.user_annotations__local_file_csv__file = default_config["user_annotations"]["local_file_csv"]["file"]
             self.user_annotations__ontology__enable = default_config["user_annotations"]["ontology"]["enable"]
             self.user_annotations__ontology__obo_location = default_config["user_annotations"]["ontology"][
                 "obo_location"
-            ]  # noqa E501
+            ]
             self.user_annotations__hosted_tiledb_array__db_uri = default_config["user_annotations"][
                 "hosted_tiledb_array"
-            ][
-                "db_uri"
-            ]  # noqa E501
+            ]["db_uri"]
             self.user_annotations__hosted_tiledb_array__hosted_file_directory = default_config["user_annotations"][
                 "hosted_tiledb_array"
-            ][
-                "hosted_file_directory"
-            ]  # noqa E501
+            ]["hosted_file_directory"]
 
             self.embeddings__names = default_config["embeddings"]["names"]
             self.embeddings__enable_reembedding = default_config["embeddings"]["enable_reembedding"]
@@ -98,20 +94,20 @@ class DatasetConfig(BaseConfig):
         self.validate_correct_type_of_configuration_attribute("user_annotations__type", str)
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__local_file_csv__directory", (type(None), str)
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__local_file_csv__file", (type(None), str)
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute("user_annotations__ontology__enable", bool)
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__ontology__obo_location", (type(None), str)
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__hosted_tiledb_array__db_uri", (type(None), str)
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__hosted_tiledb_array__hosted_file_directory", (type(None), str)
-        )  # noqa E501
+        )
         if self.user_annotations__enable:
             server_config = self.app_config.server_config
             if not self.app__authentication_enable:
@@ -166,7 +162,7 @@ class DatasetConfig(BaseConfig):
         self.validate_correct_type_of_configuration_attribute("user_annotations__hosted_tiledb_array__db_uri", str)
         self.validate_correct_type_of_configuration_attribute(
             "user_annotations__hosted_tiledb_array__hosted_file_directory", str
-        )  # noqa E501
+        )
         self.user_annotations = AnnotationsHostedTileDB(
             directory_path=self.user_annotations__hosted_tiledb_array__hosted_file_directory,
             db=DbUtils(self.user_annotations__hosted_tiledb_array__db_uri),
diff --git a/server/common/config/external_config.py b/server/common/config/external_config.py
new file mode 100644
index 00000000..bebfbfa9
--- /dev/null
+++ b/server/common/config/external_config.py
@@ -0,0 +1,96 @@
+import os
+
+from server.common.config.base_config import BaseConfig
+from server.common.errors import ConfigurationError
+from server.common.config import get_secret_key
+from server.common.errors import SecretKeyRetrievalError
+from server.common.utils.type_conversion_utils import convert_string_to_value
+
+
+class ExternalConfig(BaseConfig):
+    """Manages the config attribute associated with external configuration sources, such as
+    environment variables or the AWS Secrets Manager."""
+
+    def __init__(self, app_config, default_config):
+        super().__init__(app_config, default_config)
+        try:
+            self.environment = default_config["environment"]
+            self.aws_secrets_manager__region = default_config["aws_secrets_manager"]["region"]
+            self.aws_secrets_manager__secrets = default_config["aws_secrets_manager"]["secrets"]
+
+        except KeyError as e:
+            raise ConfigurationError(f"Unexpected config: {str(e)}")
+
+    def complete_config(self, context):
+        self.handle_environment(context)
+        self.handle_aws_secrets_manager(context)
+
+    def handle_environment(self, context):
+        """For each environment variable defined, get the value (if it is set),
+        and set the specified config parameter"""
+        self.validate_correct_type_of_configuration_attribute("environment", list)
+        for envdict in self.environment:
+            name = envdict.get("name")
+            if name is None:
+                raise ConfigurationError("environment: 'name' is missing")
+            required = envdict.get("required", False)
+            if type(required) != bool:
+                raise ConfigurationError("environment: 'required' must be a bool")
+            path = envdict.get("path")
+            if path is None:
+                raise ConfigurationError("environment: 'path' is missing")
+
+            value = os.environ.get(name)
+            if value is None:
+                if required:
+                    raise ConfigurationError(f"required environment variable '{name}' not set")
+            else:
+                value = convert_string_to_value(value)
+                self.app_config.update_single_config_from_path_and_value(path, value)
+
+    def handle_aws_secrets_manager(self, context):
+        """For each aws secret defined, get the key/values, and set the specified config parameter"""
+        self.validate_correct_type_of_configuration_attribute("aws_secrets_manager__region", (type(None), str))
+        self.validate_correct_type_of_configuration_attribute("aws_secrets_manager__secrets", list)
+
+        if not self.aws_secrets_manager__secrets:
+            return
+
+        self.validate_correct_type_of_configuration_attribute("aws_secrets_manager__region", str)
+
+        for secret in self.aws_secrets_manager__secrets:
+            secret_name = secret.get("name")
+            if secret_name is None:
+                raise ConfigurationError("aws_secrets_manager: 'name' is missing")
+            if not isinstance(secret_name, str):
+                raise ConfigurationError("aws_secrets_manager: 'name' must be a string")
+
+            try:
+                secret_dict = get_secret_key(self.aws_secrets_manager__region, secret_name)
+            except SecretKeyRetrievalError as e:
+                raise ConfigurationError(f"Unable to retrieve secret {secret_name}: {str(e)}")
+
+            values = secret.get("values")
+            if values is None:
+                raise ConfigurationError("aws_secrets_manager: 'values' is missing")
+            if not isinstance(values, list):
+                raise ConfigurationError("aws_secrets_manager: 'values' must be a list")
+
+            for value in values:
+                key = value.get("key")
+                if key is None:
+                    raise ConfigurationError(f"missing 'key' in secret values: {secret_name}")
+                path = value.get("path")
+                if path is None:
+                    raise ConfigurationError(f"missing 'path' in secret values: {secret_name}")
+                required = value.get("required", False)
+                if type(required) != bool:
+                    raise ConfigurationError(f"wrong type for 'required' in secret values: {secret_name}")
+
+                secret_value = secret_dict.get(key)
+                if secret_value is None:
+                    if required:
+                        raise ConfigurationError(f"required secret '{secret_name}:{key}' not set")
+                else:
+                    secret_value = convert_string_to_value(secret_value)
+                    self.app_config.update_single_config_from_path_and_value(path, secret_value)
diff --git a/server/common/config/server_config.py b/server/common/config/server_config.py
index 508b3d4f..6c4b5570 100644
--- a/server/common/config/server_config.py
+++ b/server/common/config/server_config.py
@@ -44,17 +44,17 @@ class ServerConfig(BaseConfig):
             self.authentication__type = default_config["authentication"]["type"]
             self.authentication__params_oauth__oauth_api_base_url = default_config["authentication"]["params_oauth"][
                 "oauth_api_base_url"
-            ]  # noqa E501
+            ]
             self.authentication__params_oauth__client_id = default_config["authentication"]["params_oauth"]["client_id"]
             self.authentication__params_oauth__client_secret = default_config["authentication"]["params_oauth"][
                 "client_secret"
-            ]  # noqa E501
+            ]
             self.authentication__params_oauth__jwt_decode_options = default_config["authentication"]["params_oauth"][
                 "jwt_decode_options"
-            ]  # noqa E501
+            ]
             self.authentication__params_oauth__session_cookie = default_config["authentication"]["params_oauth"][
                 "session_cookie"
-            ]  # noqa E501
+            ]
             self.authentication__params_oauth__cookie = default_config["authentication"]["params_oauth"]["cookie"]
 
             self.multi_dataset__dataroot = default_config["multi_dataset"]["dataroot"]
@@ -62,10 +62,10 @@ class ServerConfig(BaseConfig):
             self.multi_dataset__allowed_matrix_types = default_config["multi_dataset"]["allowed_matrix_types"]
             self.multi_dataset__matrix_cache__max_datasets = default_config["multi_dataset"]["matrix_cache"][
                 "max_datasets"
-            ]  # noqa E501
+            ]
             self.multi_dataset__matrix_cache__timelimit_s = default_config["multi_dataset"]["matrix_cache"][
                 "timelimit_s"
-            ]  # noqa E501
+            ]
 
             self.single_dataset__datapath = default_config["single_dataset"]["datapath"]
             self.single_dataset__obs_names = default_config["single_dataset"]["obs_names"]
@@ -151,11 +151,6 @@ class ServerConfig(BaseConfig):
         if not self.app__verbose:
             sys.tracebacklimit = 0
 
-        # secret key:
-        #   first, from CXG_SECRET_KEY environment variable
-        #   second, from config file
-        self.app__flask_secret_key = os.environ.get("CXG_SECRET_KEY", self.app__flask_secret_key)
-
         # CSP Directives are a dict of string: list(string) or string: string
         if self.app__csp_directives is not None:
             for k, v in self.app__csp_directives.items():
@@ -178,25 +173,20 @@ class ServerConfig(BaseConfig):
         ptypes = str if self.authentication__type == "oauth" else (type(None), str)
         self.validate_correct_type_of_configuration_attribute(
             "authentication__params_oauth__oauth_api_base_url", ptypes
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute("authentication__params_oauth__client_id", ptypes)
         self.validate_correct_type_of_configuration_attribute("authentication__params_oauth__client_secret", ptypes)
         self.validate_correct_type_of_configuration_attribute(
             "authentication__params_oauth__jwt_decode_options", (type(None), dict)
-        )  # noqa E501
+        )
         self.validate_correct_type_of_configuration_attribute("authentication__params_oauth__session_cookie", bool)
 
         if self.authentication__params_oauth__session_cookie:
             self.validate_correct_type_of_configuration_attribute(
                 "authentication__params_oauth__cookie", (type(None), dict)
-            )  # noqa E501
+            )
         else:
             self.validate_correct_type_of_configuration_attribute("authentication__params_oauth__cookie", dict)
-        #   secret key: first, from CXG_OAUTH_CLIENT_SECRET environment variable
-        #   second, from config file
-        self.authentication__params_oauth__client_secret = os.environ.get(
-            "CXG_OAUTH_CLIENT_SECRET", self.authentication__params_oauth__client_secret
-        )
 
         self.auth = AuthTypeFactory.create(self.authentication__type, self)
         if self.auth is None:
@@ -286,7 +276,7 @@ class ServerConfig(BaseConfig):
         self.validate_correct_type_of_configuration_attribute("multi_dataset__matrix_cache__max_datasets", int)
         self.validate_correct_type_of_configuration_attribute(
             "multi_dataset__matrix_cache__timelimit_s", (type(None), int, float)
-        )  # noqa E501
+        )
 
         if self.multi_dataset__dataroot is None:
             return
diff --git a/server/common/utils/type_conversion_utils.py b/server/common/utils/type_conversion_utils.py
index 4bc6bf88..ccda1177 100644
--- a/server/common/utils/type_conversion_utils.py
+++ b/server/common/utils/type_conversion_utils.py
@@ -151,3 +151,17 @@ def convert_pandas_series_to_numpy(series_to_convert: pd.Series, dtype):
         logging.error("Cannot convert a pandas Series object to an integer dtype if it contains NaNs.")
 
     return series_to_convert.to_numpy(dtype)
+
+
+def convert_string_to_value(value: str):
+    """convert a string to value with the most appropriate type"""
+    if value.lower() == "true":
+        return True
+    if value.lower() == "false":
+        return False
+    if value == "null":
+        return None
+    try:
+        return eval(value)
+    except:  # noqa E722
+        return value
diff --git a/server/default_config.py b/server/default_config.py
index 88e96fe5..20922ef9 100644
--- a/server/default_config.py
+++ b/server/default_config.py
@@ -204,6 +204,61 @@ dataset:
     enable: true
     lfc_cutoff: 0.01
     top_n: 10
+
+external:
+  # You can retrieve configuration parameters from this config file, the environment,
+  # the AWS secrets manager, or from the "cellxgene launch" command line arguments.
+  # They are applied in that order, meaning that if a parameter is defined in more
+  # than one location, the last one applied takes effect.
+
+  # environment variables:
+  # This section describes how to map environment variables to configuration parameters.
+  # The format is a list defining an environment variable.
+  # Each entry in the list is a dictionary with three entries:
+  # name:  the name of the environment variable
+  # path: the path within the cellxgene configuration to update.
+  # required: (default=False) a boolean.  If true, then it is an error if the environment variable is not set.
+
+  environment:
+     - name: CXG_SECRET_KEY
+       path: [server, app, flask_secret_key]
+       required: false
+     - name: CXG_OAUTH_CLIENT_SECRET
+       path: [server, authentication, params_oauth, client_secret]
+       required: false
+
+  # AWS Secrets Manager
+  # This section describes how to map aws secrets to configuration parameters.
+  # The format is the region for the secrets manager, then a list of secrets.
+  # each secret has a name, and a list of values.
+  # Each entry in the list of values is a dictionary with three entries:
+  # key:  the key of the aws secret.
+  # path: the path within the cellxgene configuration to update.
+  # required: (default=False) a boolean.  If true, then it is an error if the key does not exist in the secret.
+  #
+  # example:
+  # aws_secrets_manager:
+  #   region: us-west-2
+  #    - name: my_first_secret
+  #      values:
+  #      - key: flask_secret_key
+  #        path: [server, app, flask_secret_key]
+  #        required: true
+  #      - key: db_uri
+  #        path: [dataset, user_annotations, hosted_tiledb_array, db_uri]
+  #        required: true
+  #    - name:  my_auth_secret
+  #      values:
+  #      - key: client_secret
+  #        path: [server, authentication, params_oauth, client_secret]
+  #        required: true
+  #      - key: client_id
+  #        path: [server, authentication, params_oauth, client_id]
+  #        required: true
+
+  aws_secrets_manager:
+    region: null
+    secrets: []
 """
 
 
diff --git a/server/test/__init__.py b/server/test/__init__.py
index bbb1de0b..586aa798 100644
--- a/server/test/__init__.py
+++ b/server/test/__init__.py
@@ -117,7 +117,7 @@ def random_string(n):
     return "".join(random.choice(string.ascii_letters) for _ in range(n))
 
 
-def start_test_server(command_line_args=[], app_config=None):
+def start_test_server(command_line_args=[], app_config=None, env=None):
     """
     Command line arguments can be passed in, as well as an app_config.
     This function is meant to be used like this, for example:
@@ -155,7 +155,7 @@ def start_test_server(command_line_args=[], app_config=None):
         command.extend(["-c", config_file])
 
     server = f"http://localhost:{port}"
-    ps = Popen(command)
+    ps = Popen(command, env=env)
 
     for _ in range(10):
         try:
@@ -178,10 +178,10 @@ def stop_test_server(ps):
 
 
 @contextmanager
-def test_server(command_line_args=[], app_config=None):
+def test_server(command_line_args=[], app_config=None, env=None):
     """A context to run the cellxgene server."""
 
-    ps, server = start_test_server(command_line_args, app_config)
+    ps, server = start_test_server(command_line_args, app_config, env)
     try:
         yield server
     finally:
diff --git a/server/test/unit/common/config/__init__.py b/server/test/unit/common/config/__init__.py
index fa730df2..7c06b311 100644
--- a/server/test/unit/common/config/__init__.py
+++ b/server/test/unit/common/config/__init__.py
@@ -3,6 +3,7 @@ import shutil
 import unittest
 import random
 from unittest import mock
+import yaml
 
 from server.test import FIXTURES_ROOT
 
@@ -133,6 +134,9 @@ class ConfigTests(unittest.TestCase):
         enable_difexp="true",
         lfc_cutoff=0.01,
         top_n=10,
+        environment=None,
+        aws_secrets_manager_region=None,
+        aws_secrets_manager_secrets=[],
         config_file_name="app_config.yml",
     ):
         random_num = random.randrange(999999)
@@ -201,13 +205,17 @@ class ConfigTests(unittest.TestCase):
             top_n=top_n,
             config_file_name=f"temp_dataset_config_{random_num}.yml",
         )
-        with open(server_config) as server_config:
-            with open(dataset_config) as dataset_config:
-                with open(configfile, "w") as app_config_file:
-                    for line in server_config:
-                        app_config_file.write(line)
-                    for line in dataset_config:
-                        app_config_file.write(line)
+        external_config = self.custom_external_config(
+            environment=environment,
+            aws_secrets_manager_region=aws_secrets_manager_region,
+            aws_secrets_manager_secrets=aws_secrets_manager_secrets,
+            config_file_name=f"temp_external_config_{random_num}.yml",
+        )
+
+        with open(configfile, "w") as app_config_file:
+            app_config_file.write(open(server_config).read())
+            app_config_file.write(open(dataset_config).read())
+            app_config_file.write(open(external_config).read())
 
         return configfile
 
@@ -244,3 +252,32 @@ class ConfigTests(unittest.TestCase):
             dataset_config_file.write(dataset_config)
 
         return configfile
+
+    def custom_external_config(
+        self,
+        environment=None,
+        aws_secrets_manager_region=None,
+        aws_secrets_manager_secrets=[],
+        config_file_name="external_config.yaml",
+    ):
+        # set to the default if environment is None
+        if environment is None:
+            environment = [
+                dict(name="CXG_SECRET_KEY", path=["server", "app", "flask_secret_key"], required=False),
+                dict(
+                    name="CXG_OAUTH_CLIENT_SECRET",
+                    path=["server", "authentication", "params_oauth", "client_secret"],
+                    required=False,
+                ),
+            ]
+        external_config = {
+            "external": {
+                "environment": environment,
+                "aws_secrets_manager": {"region": aws_secrets_manager_region, "secrets": aws_secrets_manager_secrets},
+            }
+        }
+
+        configfile = os.path.join(self.tmp_fixtures_directory, config_file_name)
+        with open(configfile, "w") as external_config_file:
+            yaml.dump(external_config, external_config_file)
+        return configfile
diff --git a/server/test/unit/common/config/test_app_config.py b/server/test/unit/common/config/test_app_config.py
index 2dc3273d..362ab330 100644
--- a/server/test/unit/common/config/test_app_config.py
+++ b/server/test/unit/common/config/test_app_config.py
@@ -138,3 +138,77 @@ class AppConfigTest(ConfigTests):
             dataset_changes = app_config.default_dataset_config.changes_from_default()
             self.assertEqual(server_changes, [])
             self.assertEqual(dataset_changes, [("user_annotations__enable", False, True)])
+
+    def test_simple_update_single_config_from_path_and_value(self):
+        """Update a simple config parameter"""
+
+        config = AppConfig()
+        config.server_config.multi_dataset__dataroot = dict(
+            s1=dict(dataroot="my_dataroot_s1", base_url="my_baseurl_s1"),
+            s2=dict(dataroot="my_dataroot_s2", base_url="my_baseurl_s2"),
+        )
+        config.add_dataroot_config("s1")
+        config.add_dataroot_config("s2")
+
+        # test simple value in server
+        config.update_single_config_from_path_and_value(["server", "app", "flask_secret_key"], "mysecret")
+        self.assertEqual(config.server_config.app__flask_secret_key, "mysecret")
+
+        # test simple value in default dataset
+        config.update_single_config_from_path_and_value(
+            ["dataset", "user_annotations", "hosted_tiledb_array", "db_uri"], "mydburi",
+        )
+        self.assertEqual(config.default_dataset_config.user_annotations__hosted_tiledb_array__db_uri, "mydburi")
+        self.assertEqual(config.dataroot_config["s1"].user_annotations__hosted_tiledb_array__db_uri, "mydburi")
+        self.assertEqual(config.dataroot_config["s2"].user_annotations__hosted_tiledb_array__db_uri, "mydburi")
+
+        # test simple value in specific dataset
+        config.update_single_config_from_path_and_value(
+            ["per_dataset_config", "s1", "user_annotations", "hosted_tiledb_array", "db_uri"], "s1dburi"
+        )
+        self.assertEqual(config.default_dataset_config.user_annotations__hosted_tiledb_array__db_uri, "mydburi")
+        self.assertEqual(config.dataroot_config["s1"].user_annotations__hosted_tiledb_array__db_uri, "s1dburi")
+        self.assertEqual(config.dataroot_config["s2"].user_annotations__hosted_tiledb_array__db_uri, "mydburi")
+
+        # error checking
+        bad_paths = [
+            (
+                ["dataset", "does", "not", "exist"],
+                "unknown config parameter at path: '['dataset', 'does', 'not', 'exist']'",
+            ),
+            (["does", "not", "exist"], "path must start with 'server', 'dataset', or 'per_dataset_config'"),
+            ([], "path must start with 'server', 'dataset', or 'per_dataset_config'"),
+            (["per_dataset_config"], "missing dataroot when using per_dataset_config: got '['per_dataset_config']'"),
+            (
+                ["per_dataset_config", "unknown"],
+                "unknown dataroot when using per_dataset_config: got '['per_dataset_config', 'unknown']',"
+                " dataroots specified in config are ['s1', 's2']",
+            ),
+            ([1, 2, 3], "path must be a list of strings, got '[1, 2, 3]'"),
+            ("string", "path must be a list of strings, got 'string'"),
+        ]
+        for bad_path, error_message in bad_paths:
+            with self.assertRaises(ConfigurationError) as config_error:
+                config.update_single_config_from_path_and_value(bad_path, "value")
+
+            self.assertEqual(config_error.exception.message, error_message)
+
+    def test_dict_update_single_config_from_path_and_value(self):
+        """Update a config parameter that has a value of dict"""
+
+        # the path leads to a dict config param, set the config parameter to the new value
+        config = AppConfig()
+        config.update_single_config_from_path_and_value(
+            ["server", "authentication", "params_oauth", "cookie"], dict(key="mykey1", max_age=100)
+        )
+        self.assertEqual(config.server_config.authentication__params_oauth__cookie, dict(key="mykey1", max_age=100))
+
+        # the path leads to an entry within a dict config param, the value is simple
+        config = AppConfig()
+        config.server_config.authentication__params_oauth__cookie = dict(key="mykey1", max_age=100)
+        config.update_single_config_from_path_and_value(
+            ["server", "authentication", "params_oauth", "cookie", "httponly"], True,
+        )
+        self.assertEqual(
+            config.server_config.authentication__params_oauth__cookie, dict(key="mykey1", max_age=100, httponly=True)
+        )
diff --git a/server/test/unit/common/config/test_dataset_config.py b/server/test/unit/common/config/test_dataset_config.py
index a32d1f87..5ad66e44 100644
--- a/server/test/unit/common/config/test_dataset_config.py
+++ b/server/test/unit/common/config/test_dataset_config.py
@@ -155,7 +155,7 @@ class TestDatasetConfig(ConfigTests):
         # test for illegal url_dataroots
         for illegal in ("../b", "!$*", "\\n", "", "(bad)"):
             config.update_server_config(
-                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": "{PROJECT_ROOT}/example-dataset"}}
+                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}}
             )
             with self.assertRaises(ConfigurationError):
                 config.complete_config()
@@ -163,7 +163,7 @@ class TestDatasetConfig(ConfigTests):
         # test for legal url_dataroots
         for legal in ("d", "this.is-okay_", "a/b"):
             config.update_server_config(
-                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": "{PROJECT_ROOT}/example-dataset"}}
+                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}}
             )
             config.complete_config()
 
diff --git a/server/test/unit/common/config/test_external_config.py b/server/test/unit/common/config/test_external_config.py
new file mode 100644
index 00000000..5e3825e6
--- /dev/null
+++ b/server/test/unit/common/config/test_external_config.py
@@ -0,0 +1,231 @@
+import os
+from unittest.mock import patch
+
+import requests
+
+from server.common.errors import ConfigurationError
+from server.common.config.app_config import AppConfig
+from server.test import test_server, FIXTURES_ROOT
+from server.common.utils.type_conversion_utils import convert_string_to_value
+from server.test.unit.common.config import ConfigTests
+
+
+class TestExternalConfig(ConfigTests):
+    def test_type_convert(self):
+        # The values from environment variables and aws secrets are returned as strings.
+        # These values need to be converted to the proper types.
+
+        self.assertEqual(convert_string_to_value("1"), int(1))
+        self.assertEqual(convert_string_to_value("1.1"), float(1.1))
+        self.assertEqual(convert_string_to_value("string"), "string")
+        self.assertEqual(convert_string_to_value("true"), True)
+        self.assertEqual(convert_string_to_value("True"), True)
+        self.assertEqual(convert_string_to_value("false"), False)
+        self.assertEqual(convert_string_to_value("False"), False)
+        self.assertEqual(convert_string_to_value("null"), None)
+        self.assertEqual(convert_string_to_value("None"), None)
+        self.assertEqual(convert_string_to_value("{'a':10, 'b':'string'}"), dict(a=int(10), b="string"))
+
+    def test_environment_variable(self):
+        configfile = self.custom_external_config(
+            environment=[
+                dict(name="DATAPATH", path=["server", "single_dataset", "datapath"], required=True),
+                dict(name="DIFFEXP", path=["dataset", "diffexp", "enable"], required=True),
+            ],
+            config_file_name="environment_external_config.yaml",
+        )
+
+        env = os.environ
+        env["DATAPATH"] = f"{FIXTURES_ROOT}/pbmc3k.cxg"
+        env["DIFFEXP"] = "False"
+        with test_server(command_line_args=["-c", configfile], env=env) as server:
+            session = requests.Session()
+            response = session.get(f"{server}/api/v0.2/config")
+            data_config = response.json()
+            self.assertEqual(data_config["config"]["displayNames"]["dataset"], "pbmc3k")
+            self.assertTrue(data_config["config"]["parameters"]["disable-diffexp"])
+
+        env["DATAPATH"] = f"{FIXTURES_ROOT}/a95c59b4-7f5d-4b80-ad53-a694834ca18b.h5ad"
+        env["DIFFEXP"] = "True"
+        with test_server(command_line_args=["-c", configfile], env=env) as server:
+            session = requests.Session()
+            response = session.get(f"{server}/api/v0.2/config")
+            data_config = response.json()
+            self.assertEqual(data_config["config"]["displayNames"]["dataset"], "a95c59b4-7f5d-4b80-ad53-a694834ca18b")
+            self.assertFalse(data_config["config"]["parameters"]["disable-diffexp"])
+
+    def test_environment_variable_errors(self):
+
+        # no name
+        app_config = AppConfig()
+        app_config.external_config.environment = [dict(required=True, path=["this", "is", "a", "path"])]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "environment: 'name' is missing")
+
+        # required has wrong type
+        app_config = AppConfig()
+        app_config.external_config.environment = [
+            dict(name="myenvar", required="optional", path=["this", "is", "a", "path"])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "environment: 'required' must be a bool")
+
+        # no path
+        app_config = AppConfig()
+        app_config.external_config.environment = [dict(name="myenvar", required=True)]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "environment: 'path' is missing")
+
+        # required environment variable is not set
+        app_config = AppConfig()
+        app_config.external_config.environment = [
+            dict(name="THIS_ENV_IS_NOT_SET", required=True, path=["this", "is", "a", "path"])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "required environment variable 'THIS_ENV_IS_NOT_SET' not set")
+
+    @patch("server.common.config.external_config.get_secret_key")
+    def test_aws_secrets_manager(self, mock_get_secret_key):
+        mock_get_secret_key.return_value = {
+            "oauth_client_secret": "mock_oauth_secret",
+            "db_uri": "mock_db_uri",
+        }
+        configfile = self.custom_external_config(
+            aws_secrets_manager_region="us-west-2",
+            aws_secrets_manager_secrets=[
+                dict(
+                    name="my_secret",
+                    values=[
+                        dict(key="flask_secret_key", path=["server", "app", "flask_secret_key"], required=False),
+                        dict(
+                            key="db_uri",
+                            path=["dataset", "user_annotations", "hosted_tiledb_array", "db_uri"],
+                            required=True,
+                        ),
+                        dict(
+                            key="oauth_client_secret",
+                            path=["server", "authentication", "params_oauth", "client_secret"],
+                            required=True,
+                        ),
+                    ],
+                )
+            ],
+            config_file_name="secret_external_config.yaml",
+        )
+
+        app_config = AppConfig()
+        app_config.update_from_config_file(configfile)
+        app_config.server_config.single_dataset__datapath = f"{FIXTURES_ROOT}/pbmc3k.cxg"
+        app_config.server_config.app__flask_secret_key = "original"
+        app_config.server_config.single_dataset__datapath = f"{FIXTURES_ROOT}/pbmc3k.cxg"
+
+        app_config.complete_config()
+
+        self.assertEqual(app_config.server_config.app__flask_secret_key, "original")
+        self.assertEqual(app_config.server_config.authentication__params_oauth__client_secret, "mock_oauth_secret")
+        self.assertEqual(app_config.default_dataset_config.user_annotations__hosted_tiledb_array__db_uri, "mock_db_uri")
+
+    @patch("server.common.config.external_config.get_secret_key")
+    def test_aws_secrets_manager_error(self, mock_get_secret_key):
+        mock_get_secret_key.return_value = {
+            "oauth_client_secret": "mock_oauth_secret",
+            "db_uri": "mock_db_uri",
+        }
+
+        # no region
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = None
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name="secret1", values=[dict(key="key1", required=True, path=["this", "is", "my", "path"])])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(
+            config_error.exception.message,
+            "Invalid type for attribute: aws_secrets_manager__region, expected type str, got NoneType",
+        )
+
+        # missing secret name
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(values=[dict(key="db_uri", required=True, path=["this", "is", "my", "path"])])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "aws_secrets_manager: 'name' is missing")
+
+        # secret name wrong type
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name=1, values=[dict(key="db_uri", required=True, path=["this", "is", "my", "path"])])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "aws_secrets_manager: 'name' must be a string")
+
+        # missing values name
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [dict(name="mysecret")]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "aws_secrets_manager: 'values' is missing")
+
+        # values wrong type
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name="mysecret", values=dict(key="db_uri", required=True, path=["this", "is", "my", "path"]))
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "aws_secrets_manager: 'values' must be a list")
+
+        # entry missing key
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name="mysecret", values=[dict(required=True, path=["this", "is", "my", "path"])])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "missing 'key' in secret values: mysecret")
+
+        # entry required is wrong type
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name="mysecret", values=[dict(key="db_uri", required="optional", path=["this", "is", "my", "path"])])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "wrong type for 'required' in secret values: mysecret")
+
+        # entry missing path
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(name="mysecret", values=[dict(key="db_uri", required=True)])
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "missing 'path' in secret values: mysecret")
+
+        # secret missing required key
+        app_config = AppConfig()
+        app_config.external_config.aws_secrets_manager__region = "us-west-2"
+        app_config.external_config.aws_secrets_manager__secrets = [
+            dict(
+                name="mysecret",
+                values=[dict(key="KEY_DOES_NOT_EXIST", required=True, path=["this", "is", "a", "path"])],
+            )
+        ]
+        with self.assertRaises(ConfigurationError) as config_error:
+            app_config.complete_config()
+        self.assertEqual(config_error.exception.message, "required secret 'mysecret:KEY_DOES_NOT_EXIST' not set")
diff --git a/server/test/unit/common/config/test_server_config.py b/server/test/unit/common/config/test_server_config.py
index 78ee9570..65974523 100644
--- a/server/test/unit/common/config/test_server_config.py
+++ b/server/test/unit/common/config/test_server_config.py
@@ -105,13 +105,14 @@ class TestServerConfig(ConfigTests):
         self.config = AppConfig()
         self.config.server_config.handle_app(self.context)
         self.assertEqual(self.config.server_config.app__port, 4008)
+        del os.environ["CXG_SERVER_PORT"]
 
     def test_handle_app__can_get_secret_key_from_envvar_or_config_file_with_envvar_given_preference(self):
         config = self.get_config(flask_secret_key="KEY_FROM_FILE")
         self.assertEqual(config.server_config.app__flask_secret_key, "KEY_FROM_FILE")
 
         os.environ["CXG_SECRET_KEY"] = "KEY_FROM_ENV"
-        config.server_config.handle_app(self.context)
+        config.external_config.handle_environment(self.context)
         self.assertEqual(config.server_config.app__flask_secret_key, "KEY_FROM_ENV")
 
     def test_handle_app__sets_web_base_url(self):
@@ -124,7 +125,7 @@ class TestServerConfig(ConfigTests):
         self.assertEqual(config.server_config.authentication__params_oauth__client_secret, "KEY_FROM_FILE")
 
         os.environ["CXG_OAUTH_CLIENT_SECRET"] = "KEY_FROM_ENV"
-        config.server_config.handle_authentication()
+        config.external_config.handle_environment(self.context)
 
         self.assertEqual(config.server_config.authentication__params_oauth__client_secret, "KEY_FROM_ENV")
 
@@ -215,7 +216,7 @@ class TestServerConfig(ConfigTests):
         # test for illegal url_dataroots
         for illegal in ("../b", "!$*", "\\n", "", "(bad)"):
             self.config.update_server_config(
-                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": "{PROJECT_ROOT}/example-dataset"}}
+                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}}
             )
             with self.assertRaises(ConfigurationError):
                 self.config.complete_config()
@@ -224,7 +225,7 @@ class TestServerConfig(ConfigTests):
         # test for legal url_dataroots
         for legal in ("d", "this.is-okay_", "a/b"):
             self.config.update_server_config(
-                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": "{PROJECT_ROOT}/example-dataset"}}
+                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}}
             )
             self.config.complete_config()
 

From b5ec43c4b124e4b64449d41ea0f7cc6bd404196f Mon Sep 17 00:00:00 2001
From: bmccandless <bmccandless@chanzuckerberg.com>
Date: Thu, 8 Oct 2020 08:44:09 -0700
Subject: [PATCH 05/16] Add a function to check the configuration for errors.
 (#1919)

This can be used as a sanity check before a deployment:

  chanzuckerberg/single-cell#63
---
 server/common/config/server_config.py         |  2 +-
 server/eb/check_config.py                     | 39 ++++++++++++
 server/test/__init__.py                       |  8 ++-
 server/test/unit/auth/test_auth.py            | 60 ++++++++++---------
 server/test/unit/auth/test_oauth.py           |  2 +
 server/test/unit/common/config/__init__.py    |  4 +-
 .../unit/common/config/test_app_config.py     |  8 ++-
 .../unit/common/config/test_base_config.py    |  2 +
 .../unit/common/config/test_dataset_config.py | 10 +++-
 .../unit/common/config/test_server_config.py  |  3 +
 server/test/unit/eb/test_eb.py                | 39 ++++++++++--
 11 files changed, 136 insertions(+), 41 deletions(-)
 create mode 100644 server/eb/check_config.py

diff --git a/server/common/config/server_config.py b/server/common/config/server_config.py
index 6c4b5570..14eac613 100644
--- a/server/common/config/server_config.py
+++ b/server/common/config/server_config.py
@@ -114,7 +114,7 @@ class ServerConfig(BaseConfig):
         self.validate_correct_type_of_configuration_attribute("app__port", (type(None), int))
         self.validate_correct_type_of_configuration_attribute("app__open_browser", bool)
         self.validate_correct_type_of_configuration_attribute("app__force_https", bool)
-        self.validate_correct_type_of_configuration_attribute("app__flask_secret_key", (type(None), str))
+        self.validate_correct_type_of_configuration_attribute("app__flask_secret_key", str)
         self.validate_correct_type_of_configuration_attribute("app__generate_cache_control_headers", bool)
         self.validate_correct_type_of_configuration_attribute("app__server_timing_headers", bool)
         self.validate_correct_type_of_configuration_attribute("app__csp_directives", (type(None), dict))
diff --git a/server/eb/check_config.py b/server/eb/check_config.py
new file mode 100644
index 00000000..6886e101
--- /dev/null
+++ b/server/eb/check_config.py
@@ -0,0 +1,39 @@
+import sys
+import argparse
+import yaml
+
+from server.common.config.app_config import AppConfig
+
+
+def main():
+    parser = argparse.ArgumentParser("A script to check hosted configuration files")
+    parser.add_argument("config_file", help="the configuration file")
+    parser.add_argument(
+        "-s",
+        "--show",
+        default=False,
+        action="store_true",
+        help="print the configuration. NOTE: this may print secret values to stdout",
+    )
+
+    args = parser.parse_args()
+
+    app_config = AppConfig()
+    app_config.update_from_config_file(args.config_file)
+    try:
+        app_config.complete_config()
+    except Exception as e:
+        print(f"Error: {str(e)}")
+        print("FAIL:", args.config_file)
+        sys.exit(1)
+
+    if args.show:
+        yaml_config = app_config.config_to_dict()
+        yaml.dump(yaml_config, sys.stdout)
+
+    print("PASS:", args.config_file)
+    sys.exit(0)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/server/test/__init__.py b/server/test/__init__.py
index 586aa798..1b96f7c1 100644
--- a/server/test/__init__.py
+++ b/server/test/__init__.py
@@ -34,7 +34,7 @@ def data_with_tmp_tiledb_annotations(ext: MatrixDataType):
     data_locator = DataLocator(fname)
     config = AppConfig()
     config.update_server_config(
-        multi_dataset__dataroot=data_locator.path, authentication__type="test",
+        app__flask_secret_key="secret", multi_dataset__dataroot=data_locator.path, authentication__type="test",
     )
     config.update_default_dataset_config(
         embeddings__names=["umap"],
@@ -64,7 +64,10 @@ def data_with_tmp_annotations(ext: MatrixDataType, annotations_fixture=False):
     data_locator = DataLocator(fname)
     config = AppConfig()
     config.update_server_config(
-        single_dataset__obs_names=None, single_dataset__var_names=None, single_dataset__datapath=data_locator.path
+        app__flask_secret_key="secret",
+        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,
@@ -97,6 +100,7 @@ def skip_if(condition, reason: str):
 def app_config(data_locator, backed=False, extra_server_config={}, extra_dataset_config={}):
     config = AppConfig()
     config.update_server_config(
+        app__flask_secret_key="secret",
         single_dataset__obs_names=None,
         single_dataset__var_names=None,
         adaptor__anndata_adaptor__backed=backed,
diff --git a/server/test/unit/auth/test_auth.py b/server/test/unit/auth/test_auth.py
index 0b8c0bf1..2caba6b1 100644
--- a/server/test/unit/auth/test_auth.py
+++ b/server/test/unit/auth/test_auth.py
@@ -11,13 +11,14 @@ class AuthTest(unittest.TestCase):
         self.dataset_dataroot = FIXTURES_ROOT
 
     def test_auth_none(self):
-        c = AppConfig()
-        c.update_server_config(authentication__type=None, multi_dataset__dataroot=self.dataset_dataroot)
-        c.update_default_dataset_config(user_annotations__enable=False)
+        app_config = AppConfig()
+        app_config.update_server_config(app__flask_secret_key="secret")
+        app_config.update_server_config(authentication__type=None, multi_dataset__dataroot=self.dataset_dataroot)
+        app_config.update_default_dataset_config(user_annotations__enable=False)
 
-        c.complete_config()
+        app_config.complete_config()
 
-        with test_server(app_config=c) as server:
+        with test_server(app_config=app_config) as server:
             session = requests.Session()
             config = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config").json()
             userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json()
@@ -25,12 +26,13 @@ class AuthTest(unittest.TestCase):
             self.assertIsNone(userinfo)
 
     def test_auth_session(self):
-        c = AppConfig()
-        c.update_server_config(authentication__type="session", multi_dataset__dataroot=self.dataset_dataroot)
-        c.update_default_dataset_config(user_annotations__enable=True)
-        c.complete_config()
+        app_config = AppConfig()
+        app_config.update_server_config(app__flask_secret_key="secret")
+        app_config.update_server_config(authentication__type="session", multi_dataset__dataroot=self.dataset_dataroot)
+        app_config.update_default_dataset_config(user_annotations__enable=True)
+        app_config.complete_config()
 
-        with test_server(app_config=c) as server:
+        with test_server(app_config=app_config) as server:
             session = requests.Session()
             config = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config").json()
             userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json()
@@ -40,9 +42,10 @@ class AuthTest(unittest.TestCase):
             self.assertEqual(userinfo["userinfo"]["username"], "anonymous")
 
     def test_auth_test(self):
-        c = AppConfig()
-        c.update_server_config(authentication__type="test")
-        c.update_server_config(
+        app_config = AppConfig()
+        app_config.update_server_config(app__flask_secret_key="secret")
+        app_config.update_server_config(authentication__type="test")
+        app_config.update_server_config(
             multi_dataset__dataroot=dict(
                 a1=dict(dataroot=self.dataset_dataroot, base_url="auth"),
                 a2=dict(dataroot=self.dataset_dataroot, base_url="no-auth"),
@@ -50,12 +53,12 @@ class AuthTest(unittest.TestCase):
         )
 
         # specialize the configs
-        c.add_dataroot_config("a1", app__authentication_enable=True, user_annotations__enable=True)
-        c.add_dataroot_config("a2", app__authentication_enable=False, user_annotations__enable=False)
+        app_config.add_dataroot_config("a1", app__authentication_enable=True, user_annotations__enable=True)
+        app_config.add_dataroot_config("a2", app__authentication_enable=False, user_annotations__enable=False)
 
-        c.complete_config()
+        app_config.complete_config()
 
-        with test_server(app_config=c) as server:
+        with test_server(app_config=app_config) as server:
             session = requests.Session()
 
             # auth datasets
@@ -102,20 +105,21 @@ class AuthTest(unittest.TestCase):
             self.assertFalse(config["config"]["parameters"]["annotations"])
 
             # login with a picture
-            r = session.get(f"{server}/{login_uri}&picture=myimage.png")
+            session.get(f"{server}/{login_uri}&picture=myimage.png")
             userinfo = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/userinfo").json()
             self.assertTrue(userinfo["userinfo"]["is_authenticated"])
             self.assertEqual(userinfo["userinfo"]["picture"], "myimage.png")
 
     def test_auth_test_single(self):
-        c = AppConfig()
-        c.update_server_config(
+        app_config = AppConfig()
+        app_config.update_server_config(app__flask_secret_key="secret")
+        app_config.update_server_config(
             authentication__type="test", single_dataset__datapath=f"{self.dataset_dataroot}/pbmc3k.cxg"
         )
 
-        c.complete_config()
+        app_config.complete_config()
 
-        with test_server(app_config=c) as server:
+        with test_server(app_config=app_config) as server:
             session = requests.Session()
             config = session.get(f"{server}/api/v0.2/config").json()
             userinfo = session.get(f"{server}/api/v0.2/userinfo").json()
@@ -130,10 +134,10 @@ class AuthTest(unittest.TestCase):
             self.assertEqual(login_uri, "/login")
             self.assertEqual(logout_uri, "/logout")
 
-            r = session.get(f"{server}/{login_uri}")
+            response = session.get(f"{server}/{login_uri}")
             # check that the login redirect worked
-            self.assertEqual(r.history[0].status_code, 302)
-            self.assertEqual(r.url, f"{server}/")
+            self.assertEqual(response.history[0].status_code, 302)
+            self.assertEqual(response.url, f"{server}/")
 
             config = session.get(f"{server}/api/v0.2/config").json()
             userinfo = session.get(f"{server}/api/v0.2/userinfo").json()
@@ -141,10 +145,10 @@ class AuthTest(unittest.TestCase):
             self.assertEqual(userinfo["userinfo"]["username"], "test_account")
             self.assertTrue(config["config"]["parameters"]["annotations"])
 
-            r = session.get(f"{server}/{logout_uri}")
+            response = session.get(f"{server}/{logout_uri}")
             # check that the logout redirect worked
-            self.assertEqual(r.history[0].status_code, 302)
-            self.assertEqual(r.url, f"{server}/")
+            self.assertEqual(response.history[0].status_code, 302)
+            self.assertEqual(response.url, f"{server}/")
             config = session.get(f"{server}/api/v0.2/config").json()
             userinfo = session.get(f"{server}/api/v0.2/userinfo").json()
             self.assertFalse(userinfo["userinfo"]["is_authenticated"])
diff --git a/server/test/unit/auth/test_oauth.py b/server/test/unit/auth/test_oauth.py
index 8b250ed6..f5a85cb5 100644
--- a/server/test/unit/auth/test_oauth.py
+++ b/server/test/unit/auth/test_oauth.py
@@ -160,12 +160,14 @@ class AuthTest(unittest.TestCase):
     def test_auth_oauth_session(self):
         # test with session cookies
         app_config = AppConfig()
+        app_config.update_server_config(app__flask_secret_key="secret")
         app_config.update_server_config(authentication__params_oauth__session_cookie=True,)
         self.auth_flow(app_config)
 
     def test_auth_oauth_cookie(self):
         # test with specified cookie
         app_config = AppConfig()
+        app_config.update_server_config(app__flask_secret_key="secret")
         app_config.update_server_config(
             authentication__params_oauth__session_cookie=False,
             authentication__params_oauth__cookie=dict(key="test_cxguser", httponly=True, max_age=60),
diff --git a/server/test/unit/common/config/__init__.py b/server/test/unit/common/config/__init__.py
index 7c06b311..8a48be5e 100644
--- a/server/test/unit/common/config/__init__.py
+++ b/server/test/unit/common/config/__init__.py
@@ -31,7 +31,7 @@ class ConfigTests(unittest.TestCase):
         port="null",
         open_browser="false",
         force_https="false",
-        flask_secret_key="null",
+        flask_secret_key="secret",
         generate_cache_control_headers="false",
         server_timing_headers="false",
         csp_directives="null",
@@ -82,7 +82,7 @@ class ConfigTests(unittest.TestCase):
         port="null",
         open_browser="false",
         force_https="false",
-        flask_secret_key="null",
+        flask_secret_key="secret",
         generate_cache_control_headers="false",
         server_timing_headers="false",
         csp_directives="null",
diff --git a/server/test/unit/common/config/test_app_config.py b/server/test/unit/common/config/test_app_config.py
index 362ab330..ea8fe047 100644
--- a/server/test/unit/common/config/test_app_config.py
+++ b/server/test/unit/common/config/test_app_config.py
@@ -15,6 +15,7 @@ class AppConfigTest(ConfigTests):
     def setUp(self):
         self.config_file_name = f"{unittest.TestCase.id(self).split('.')[-1]}.yml"
         self.config = AppConfig()
+        self.config.update_server_config(app__flask_secret_key="secret")
         self.config.update_server_config(multi_dataset__dataroot=FIXTURES_ROOT)
         self.server_config = self.config.server_config
         self.config.complete_config()
@@ -106,6 +107,8 @@ class AppConfigTest(ConfigTests):
             with open(configfile, "w") as fconfig:
                 config = """
                 server:
+                    app:
+                        flask_secret_key: secret
                     multi_dataset:
                         dataroot: test_dataroot
 
@@ -116,7 +119,10 @@ class AppConfigTest(ConfigTests):
             app_config.update_from_config_file(configfile)
             server_changes = app_config.server_config.changes_from_default()
             dataset_changes = app_config.default_dataset_config.changes_from_default()
-            self.assertEqual(server_changes, [("multi_dataset__dataroot", "test_dataroot", None)])
+            self.assertEqual(
+                server_changes,
+                [("app__flask_secret_key", "secret", None), ("multi_dataset__dataroot", "test_dataroot", None)],
+            )
             self.assertEqual(dataset_changes, [])
 
     def test_configfile_no_server_section(self):
diff --git a/server/test/unit/common/config/test_base_config.py b/server/test/unit/common/config/test_base_config.py
index d41082cf..d5d538cd 100644
--- a/server/test/unit/common/config/test_base_config.py
+++ b/server/test/unit/common/config/test_base_config.py
@@ -10,6 +10,7 @@ class BaseConfigTest(ConfigTests):
     def setUp(self):
         self.config_file_name = f"{unittest.TestCase.id(self).split('.')[-1]}.yml"
         self.config = AppConfig()
+        self.config.update_server_config(app__flask_secret_key="secret")
         self.config.update_server_config(multi_dataset__dataroot=FIXTURES_ROOT)
         self.server_config = self.config.server_config
         self.config.complete_config()
@@ -47,6 +48,7 @@ class BaseConfigTest(ConfigTests):
             server_changes,
             [
                 ("app__verbose", True, False),
+                ("app__flask_secret_key", "secret", None),
                 ("multi_dataset__dataroot", FIXTURES_ROOT, None),
                 ("multi_dataset__matrix_cache__timelimit_s", 5, 30),
                 ("data_locator__s3__region_name", "us-east-1", True),
diff --git a/server/test/unit/common/config/test_dataset_config.py b/server/test/unit/common/config/test_dataset_config.py
index 5ad66e44..b65c7d83 100644
--- a/server/test/unit/common/config/test_dataset_config.py
+++ b/server/test/unit/common/config/test_dataset_config.py
@@ -19,6 +19,7 @@ class TestDatasetConfig(ConfigTests):
     def setUp(self):
         self.config_file_name = f"{unittest.TestCase.id(self).split('.')[-1]}.yml"
         self.config = AppConfig()
+        self.config.update_server_config(app__flask_secret_key="secret")
         self.config.update_server_config(multi_dataset__dataroot=FIXTURES_ROOT)
         self.dataset_config = self.config.default_dataset_config
         self.config.complete_config()
@@ -155,7 +156,8 @@ class TestDatasetConfig(ConfigTests):
         # test for illegal url_dataroots
         for illegal in ("../b", "!$*", "\\n", "", "(bad)"):
             config.update_server_config(
-                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}}
+                app__flask_secret_key="secret",
+                multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}},
             )
             with self.assertRaises(ConfigurationError):
                 config.complete_config()
@@ -163,17 +165,19 @@ class TestDatasetConfig(ConfigTests):
         # test for legal url_dataroots
         for legal in ("d", "this.is-okay_", "a/b"):
             config.update_server_config(
-                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}}
+                app__flask_secret_key="secret",
+                multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": f"{PROJECT_ROOT}/example-dataset"}},
             )
             config.complete_config()
 
         # test that multi dataroots work end to end
         config.update_server_config(
+            app__flask_secret_key="secret",
             multi_dataset__dataroot=dict(
                 s1=dict(dataroot=f"{PROJECT_ROOT}/example-dataset", base_url="set1/1/2"),
                 s2=dict(dataroot=f"{FIXTURES_ROOT}", base_url="set2"),
                 s3=dict(dataroot=f"{FIXTURES_ROOT}", base_url="set3"),
-            )
+            ),
         )
 
         # Change this default to test if the dataroot overrides below work.
diff --git a/server/test/unit/common/config/test_server_config.py b/server/test/unit/common/config/test_server_config.py
index 65974523..f862a40f 100644
--- a/server/test/unit/common/config/test_server_config.py
+++ b/server/test/unit/common/config/test_server_config.py
@@ -23,6 +23,7 @@ class TestServerConfig(ConfigTests):
     def setUp(self):
         self.config_file_name = f"{unittest.TestCase.id(self).split('.')[-1]}.yml"
         self.config = AppConfig()
+        self.config.update_server_config(app__flask_secret_key="secret")
         self.config.update_server_config(multi_dataset__dataroot=FIXTURES_ROOT)
         self.server_config = self.config.server_config
         self.config.complete_config()
@@ -103,6 +104,7 @@ class TestServerConfig(ConfigTests):
         # Note if the port is set in the config file it will NOT be overwritten by a different envvar
         os.environ["CXG_SERVER_PORT"] = "4008"
         self.config = AppConfig()
+        self.config.update_server_config(app__flask_secret_key="secret")
         self.config.server_config.handle_app(self.context)
         self.assertEqual(self.config.server_config.app__port, 4008)
         del os.environ["CXG_SERVER_PORT"]
@@ -152,6 +154,7 @@ class TestServerConfig(ConfigTests):
         config = AppConfig()
         backend_port = find_available_port("localhost", 10000)
         config.update_server_config(
+            app__flask_secret_key="secret",
             app__api_base_url=f"http://localhost:{backend_port}/additional/path",
             multi_dataset__dataroot=f"{PROJECT_ROOT}/example-dataset",
         )
diff --git a/server/test/unit/eb/test_eb.py b/server/test/unit/eb/test_eb.py
index c148f6bc..61cf1867 100644
--- a/server/test/unit/eb/test_eb.py
+++ b/server/test/unit/eb/test_eb.py
@@ -6,6 +6,7 @@ from server.test import PROJECT_ROOT, FIXTURES_ROOT
 from server.common.config.app_config import AppConfig
 from contextlib import contextmanager
 import time
+import os
 
 
 @contextmanager
@@ -34,12 +35,12 @@ class Elastic_Beanstalk_Test(unittest.TestCase):
         tempdir = tempfile.TemporaryDirectory(dir=f"{PROJECT_ROOT}/server")
         tempdirname = tempdir.name
 
-        c = AppConfig()
+        config = AppConfig()
         # test that eb works
-        c.update_server_config(multi_dataset__dataroot=f"{FIXTURES_ROOT}", app__flask_secret_key="open sesame")
+        config.update_server_config(multi_dataset__dataroot=f"{FIXTURES_ROOT}", app__flask_secret_key="open sesame")
 
-        c.complete_config()
-        c.write_config(f"{tempdirname}/config.yaml")
+        config.complete_config()
+        config.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)
@@ -50,3 +51,33 @@ class Elastic_Beanstalk_Test(unittest.TestCase):
             r = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config")
             data_config = r.json()
             assert data_config["config"]["displayNames"]["dataset"] == "pbmc3k"
+
+    def test_config(self):
+        check_config_script = os.path.join(PROJECT_ROOT, "server", "eb", "check_config.py")
+        with tempfile.TemporaryDirectory() as tempdir:
+            configfile = os.path.join(tempdir, "config.yaml")
+            app_config = AppConfig()
+            app_config.update_server_config(multi_dataset__dataroot=f"{FIXTURES_ROOT}")
+            app_config.write_config(configfile)
+
+            command = ["python", check_config_script, configfile]
+
+            # test failure mode (flask_secret_key not set)
+            env = os.environ.copy()
+            env.pop("CXG_SECRET_KEY", None)
+            with self.assertRaises(subprocess.CalledProcessError) as exception_context:
+                subprocess.check_output(command, env=env)
+            output = str(exception_context.exception.stdout, "utf-8")
+            self.assertTrue(
+                output.startswith(
+                    "Error: Invalid type for attribute: app__flask_secret_key, expected type str, got NoneType"
+                )
+            )
+            self.assertEqual(exception_context.exception.returncode, 1)
+
+            # test passing case
+            env = os.environ.copy()
+            env["CXG_SECRET_KEY"] = "secret"
+            output = subprocess.check_output(command, env=env)
+            output = str(output, "utf-8")
+            self.assertTrue(output.startswith("PASS"))

From c4c48b9a5742e4fa07623002640200dc1eaeac43 Mon Sep 17 00:00:00 2001
From: Severiano Badajoz <sbadajoz@chanzuckerberg.com>
Date: Thu, 8 Oct 2020 11:02:40 -0600
Subject: [PATCH 06/16] create e2e test for auth buttons (#1907)

This PR adds a few helpful additions regarding authentication.

Changes:
* e2e tests are now run on test_oauth via a passed config.yaml
* node dev server correctly handles `/login` and `/logout` endpoints to make developing for auth easier
* Introduced auth e2e tests to check that buttons display and work
---
 client/Makefile                              |  4 +-
 client/__tests__/e2e/cellxgeneActions.js     | 11 +++--
 client/__tests__/e2e/e2e.test.js             | 12 +++++
 client/__tests__/e2e/test_config.yaml        | 47 ++++++++++++++++++++
 client/server/development.js                 | 30 +++++++++----
 client/src/components/menubar/authButtons.js |  6 ++-
 6 files changed, 94 insertions(+), 16 deletions(-)
 create mode 100644 client/__tests__/e2e/test_config.yaml

diff --git a/client/Makefile b/client/Makefile
index 3b36337a..eb5c1a78 100644
--- a/client/Makefile
+++ b/client/Makefile
@@ -3,6 +3,8 @@ include ../common.mk
 ANNOTATIONS := $(if $(ANNOTATIONS),$(ANNOTATIONS),../server/test/fixtures/pbmc3k-annotations.csv)
 ANNOTATIONS_FILENAME := $(shell basename $(ANNOTATIONS))
 
+CXG_CONFIG := $(if $(CXG_CONFIG), $(CXG_CONFIG), ./__tests__/e2e/test_config.yaml)
+
 # Packaging
 .PHONY: clean
 clean:
@@ -31,7 +33,7 @@ start-frontend:
 .PHONY: smoke-test
 smoke-test:
 	start_server_and_test \
-		'CXG_OPTIONS="--disable-annotations" $(MAKE) start-server' \
+		'CXG_OPTIONS="--config-file $(CXG_CONFIG)" $(MAKE) start-server' \
 		$(CXG_SERVER_PORT) \
 		'CXG_URL_BASE="http://localhost:$(CXG_SERVER_PORT)" npm run e2e -- --verbose false'
 
diff --git a/client/__tests__/e2e/cellxgeneActions.js b/client/__tests__/e2e/cellxgeneActions.js
index 9fda69b1..7ffcc8cf 100644
--- a/client/__tests__/e2e/cellxgeneActions.js
+++ b/client/__tests__/e2e/cellxgeneActions.js
@@ -322,7 +322,7 @@ export async function login() {
 
   await goToPage(appUrlBase);
 
-  await clickOn("auth-button");
+  await clickOn("log-in");
 
   // (thuang): Auth0 form is unstable and unsafe for input until verified
   await waitUntilFormFieldStable('[name="email"]');
@@ -341,16 +341,15 @@ export async function login() {
 }
 
 export async function logout() {
-  await clickOnUntil("menu", async () => {
-    await expect(page).toMatch("Log Out");
-
+  await clickOnUntil("user-info", async () => {
+    await waitByID("log-out");
     await Promise.all([
       page.waitForNavigation({ waitUntil: "networkidle0" }),
-      expect(page).toClick("a", { text: "Log Out" }),
+      clickOn("log-out"),
     ]);
   });
 
-  await expect(page).toMatch("Log In");
+  await waitByID("log-in");
 }
 
 async function waitUntilFormFieldStable(selector) {
diff --git a/client/__tests__/e2e/e2e.test.js b/client/__tests__/e2e/e2e.test.js
index 05f52314..2bd8ea8c 100644
--- a/client/__tests__/e2e/e2e.test.js
+++ b/client/__tests__/e2e/e2e.test.js
@@ -17,6 +17,7 @@ import {
   goToPage,
   typeInto,
   waitByID,
+  clickOnUntil,
 } from "./puppeteerUtils";
 
 import {
@@ -521,6 +522,17 @@ test("lasso moves after pan", async () => {
   expect(panCount).toBe(initialCount);
 });
 
+describe("auth buttons", () => {
+  test("login then logout", async () => {
+    await goToPage(appUrlBase);
+    await clickOnUntil("log-in", async () => {
+      await page.waitForNavigation({ waitUntil: "networkidle0" });
+      await waitByID("user-info");
+    });
+    await logout();
+  });
+});
+
 const conditionalDescribe =
   process.env.TEST_AUTH_INTEGRATION === "true" ? describe : describe.skip;
 
diff --git a/client/__tests__/e2e/test_config.yaml b/client/__tests__/e2e/test_config.yaml
new file mode 100644
index 00000000..11576878
--- /dev/null
+++ b/client/__tests__/e2e/test_config.yaml
@@ -0,0 +1,47 @@
+server:
+  app:
+    force_https: true
+
+    # By default, cellxgene will serve api requests from the same base url as the webpage.
+    # In general api_base_url and web_base_url will not need to be set.
+    # There are two reasons to set these parameters:
+    #  1. Oauth authentication is used; the oauth server will redirect back to the api_base_url after login,
+    #     which then redirects back to the web_base_url.  If the web_base_url is not set, it will default to
+    #     the api_base_url.  If oauth authentication is used, the api_base_url must be set.
+    #     For a local test (where the server runs on "http://localhost:<port>"), then the api_base_url may be
+    #     set to the string "local".
+    #  2. The cellxgene deploymnent is in an environment where the webpage and api have
+    #     different base urls.  In this case both api_base_url and web_base_url must be set.
+    #     It is up to the server admin to ensure that the networking is setup correctly for this environment.
+    api_base_url: http://localhost:5005
+    web_base_url: http://localhost:3000
+
+  authentication:
+    # The authentication types may be "none", "session", "oauth"
+    # none:  No authentication support, features like user_annotations must not be enabled.
+    # session:  A session based userid is automatically generated. (no params needed)
+    # oauth: oauth2 is used for authentication;  parameters are defined in params_oauth.
+    type: test
+
+dataset:
+  app:
+    about_legal_tos: null
+    about_legal_privacy: null
+
+  presentation:
+    max_categories: 1000
+    custom_colors: true
+
+  user_annotations:
+    enable: false
+    type: local_file_csv
+    local_file_csv:
+      directory: null
+      file: null
+    ontology:
+      enable: false
+      obo_location: null
+
+  embeddings:
+    names: []
+    enable_reembedding: false
diff --git a/client/server/development.js b/client/server/development.js
index 0b3e3811..1530856e 100644
--- a/client/server/development.js
+++ b/client/server/development.js
@@ -1,5 +1,3 @@
-const path = require("path");
-const historyApiFallback = require("connect-history-api-fallback");
 const chalk = require("chalk");
 const express = require("express");
 const favicon = require("serve-favicon");
@@ -11,35 +9,51 @@ const utils = require("./utils");
 process.env.NODE_ENV = "development";
 
 const CLIENT_PORT = process.env.CXG_CLIENT_PORT;
+const { CXG_SERVER_PORT } = process.env;
+
+const API = {
+  prefix: `http://localhost:${CXG_SERVER_PORT}/`,
+};
 
 // Set up compiler
 const compiler = webpack(config);
 
-compiler.plugin("invalid", () => {
+compiler.hooks.invalid.tap("invalid", () => {
   utils.clearConsole();
   console.log("Compiling...");
 });
 
-compiler.plugin("done", (stats) => {
+compiler.hooks.done.tap("done", (stats) => {
   utils.formatStats(stats, CLIENT_PORT);
 });
 
 // Launch server
 const app = express();
 
-app.use(historyApiFallback({ verbose: false }));
-
 app.use(
   devMiddleware(compiler, {
     logLevel: "warn",
     publicPath: config.output.publicPath,
+    index: true,
   })
 );
 
 app.use(favicon("./favicon.png"));
 
-app.get("*", (req, res) => {
-  res.sendFile(path.resolve("index.html"));
+app.get("/login", async (req, res) => {
+  try {
+    res.redirect(`${API.prefix}login?dataset=http://localhost:${CLIENT_PORT}`);
+  } catch (err) {
+    console.error(err);
+  }
+});
+
+app.get("/logout", async (req, res) => {
+  try {
+    res.redirect(`${API.prefix}logout?dataset=http://localhost:${CLIENT_PORT}`);
+  } catch (err) {
+    console.error(err);
+  }
 });
 
 app.listen(CLIENT_PORT, (err) => {
diff --git a/client/src/components/menubar/authButtons.js b/client/src/components/menubar/authButtons.js
index 60ace82b..129125ec 100644
--- a/client/src/components/menubar/authButtons.js
+++ b/client/src/components/menubar/authButtons.js
@@ -51,7 +51,11 @@ const Auth = React.memo((props) => {
 
     return (
       <Popover content={PopoverContent}>
-        <Button className={styles.menubarButton} style={{ padding: 0 }}>
+        <Button
+          data-testid="user-info"
+          className={styles.menubarButton}
+          style={{ padding: 0 }}
+        >
           {userinfo?.picture ? (
             <img alt="profile" src={userinfo?.picture} />
           ) : (

From 6677d0de568da3046babfb35bc3054cef756a487 Mon Sep 17 00:00:00 2001
From: Severiano Badajoz <sbadajoz@chanzuckerberg.com>
Date: Thu, 8 Oct 2020 12:16:02 -0600
Subject: [PATCH 07/16] disable profile picture (#1923)

---
 client/src/components/menubar/authButtons.js | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/client/src/components/menubar/authButtons.js b/client/src/components/menubar/authButtons.js
index 129125ec..a9ad8176 100644
--- a/client/src/components/menubar/authButtons.js
+++ b/client/src/components/menubar/authButtons.js
@@ -56,8 +56,9 @@ const Auth = React.memo((props) => {
           className={styles.menubarButton}
           style={{ padding: 0 }}
         >
-          {userinfo?.picture ? (
-            <img alt="profile" src={userinfo?.picture} />
+          {/*  eslint-disable-next-line no-constant-condition -- disable profile picture until CSP is tweaked */}
+          {userinfo?.picture && false ? (
+            <img alt="profile" size="21px" src={userinfo?.picture} />
           ) : (
             <span style={{ fontSize: "18px" }}>{scientist}</span>
           )}

From c01a2c72b6334b21240df898a8a9543e78e74dad Mon Sep 17 00:00:00 2001
From: Timmy Huang <tihuan@users.noreply.github.com>
Date: Thu, 8 Oct 2020 16:57:53 -0700
Subject: [PATCH 08/16] thuang-1840-authn-prompt (#1911)

---
 client/src/components/menubar/authButtons.js | 96 +++++++++++++++++++-
 client/src/components/termsPrompt/index.js   | 27 +-----
 client/src/components/util/localStorage.js   | 22 +++++
 3 files changed, 117 insertions(+), 28 deletions(-)
 create mode 100644 client/src/components/util/localStorage.js

diff --git a/client/src/components/menubar/authButtons.js b/client/src/components/menubar/authButtons.js
index a9ad8176..91729679 100644
--- a/client/src/components/menubar/authButtons.js
+++ b/client/src/components/menubar/authButtons.js
@@ -1,4 +1,5 @@
-import React from "react";
+import React, { useState } from "react";
+
 import {
   AnchorButton,
   Button,
@@ -6,20 +7,36 @@ import {
   Tooltip,
   Popover,
   Menu,
+  Elevation,
+  PopoverPosition,
+  Checkbox,
+  Card,
 } from "@blueprintjs/core";
+
 import { IconNames } from "@blueprintjs/icons";
 
 import * as globals from "../../globals";
+
 import styles from "./menubar.css";
 
+import { storageGet, storageSet, KEYS } from "../util/localStorage";
+
 const BASE_EMOJI = [0x1f9d1, 0x1f468, 0x1f469];
 const SKIN_TONES = [0x1f3fb, 0x1f3fc, 0x1f3fd, 0x1f3fe, 0x1f3ff];
 const MICROSCOPE = 0x1f52c;
 const ZERO_WIDTH_JOINER = 0x0200d;
 
+const LOGIN_PROMPT_OFF = "off";
+
 const Auth = React.memo((props) => {
+  const [isPromptOpen, setIsPromptOpen] = useState(shouldShowPrompt());
+
   const { auth, userinfo } = props;
 
+  const isAuthenticated = userinfo && userinfo.is_authenticated;
+
+  window.userinfo = userinfo;
+
   const randomInt = Math.random() * 15;
   const sexIndex = Math.floor(randomInt / 5);
   const skinToneIndex = Math.floor(randomInt % 5);
@@ -31,9 +48,9 @@ const Auth = React.memo((props) => {
     MICROSCOPE
   );
 
-  if (!auth?.["requires_client_login"]) return null;
+  if (!shouldShowAuth()) return null;
 
-  if (userinfo?.["is_authenticated"]) {
+  if (isAuthenticated) {
     const PopoverContent = (
       <Menu>
         <MenuItem
@@ -67,7 +84,7 @@ const Auth = React.memo((props) => {
     );
   }
 
-  return (
+  const LoginButton = (
     <Tooltip
       content="Log in to cellxgene"
       position="bottom"
@@ -76,7 +93,6 @@ const Auth = React.memo((props) => {
       <AnchorButton
         type="button"
         data-testid="log-in"
-        disabled={false}
         href={auth.login}
         className={styles.menubarButton}
       >
@@ -84,6 +100,76 @@ const Auth = React.memo((props) => {
       </AnchorButton>
     </Tooltip>
   );
+
+  if (isPromptOpen) {
+    return (
+      <Popover
+        position={PopoverPosition.AUTO_END}
+        isOpen
+        content={<PromptContent setIsPromptOpen={setIsPromptOpen} />}
+        onInteraction={setIsPromptOpen}
+      >
+        {LoginButton}
+      </Popover>
+    );
+  }
+
+  return LoginButton;
+
+  function shouldShowAuth() {
+    return auth && auth.requires_client_login;
+  }
+
+  function shouldShowPrompt() {
+    if (storageGet(KEYS.LOGIN_PROMPT) === LOGIN_PROMPT_OFF) return false;
+
+    return shouldShowAuth && !isAuthenticated;
+  }
 });
 
+function PromptContent({ setIsPromptOpen }) {
+  const [isChecked, setIsChecked] = useState(false);
+
+  function handleOKClick() {
+    if (isChecked) {
+      storageSet(KEYS.LOGIN_PROMPT, LOGIN_PROMPT_OFF);
+    }
+
+    setIsPromptOpen(false);
+  }
+
+  function handleCheckboxChange() {
+    setIsChecked(!isChecked);
+  }
+
+  return (
+    <Card style={{ width: "500px" }} elevation={Elevation.TWO}>
+      <p>
+        Logging in will enable you to create your own categories and labels.
+        Logging in later will reset cellxgene to the default view and cause you
+        to lose progress.
+      </p>
+      <Checkbox
+        style={{ width: "230px" }}
+        checked={isChecked}
+        onChange={handleCheckboxChange}
+        data-testid="login-hint-do-not-show-again"
+      >
+        Do not show me this message again
+      </Checkbox>
+      <div
+        style={{ display: "flex", justifyContent: "flex-end", marginTop: 15 }}
+      >
+        <Button
+          onClick={handleOKClick}
+          intent="primary"
+          data-testid="login-hint-yes"
+        >
+          Acknowledge
+        </Button>
+      </div>
+    </Card>
+  );
+}
+
 export default Auth;
diff --git a/client/src/components/termsPrompt/index.js b/client/src/components/termsPrompt/index.js
index c6209a00..9aef013c 100644
--- a/client/src/components/termsPrompt/index.js
+++ b/client/src/components/termsPrompt/index.js
@@ -8,26 +8,7 @@ import {
   Colors,
   Icon,
 } from "@blueprintjs/core";
-
-const CookieDecision = "cxg.cookieDecision";
-
-function storageGet(key, defaultValue = null) {
-  try {
-    const val = window.localStorage.getItem(key);
-    if (val === null) return defaultValue;
-    return val;
-  } catch (e) {
-    return defaultValue;
-  }
-}
-
-function storageSet(key, value) {
-  try {
-    window.localStorage.setItem(key, value);
-  } catch {
-    // continue
-  }
-}
+import { storageGet, storageSet, KEYS } from "../util/localStorage";
 
 @connect((state) => ({
   tosURL: state.config?.parameters?.["about_legal_tos"],
@@ -37,7 +18,7 @@ class TermsPrompt extends React.PureComponent {
   constructor(props) {
     super(props);
     const { tosURL, privacyURL } = this.props;
-    const cookieDecision = storageGet(CookieDecision, null);
+    const cookieDecision = storageGet(KEYS.COOKIE_DECISION, null);
     const hasDecided = cookieDecision !== null;
     this.state = {
       hasDecided,
@@ -55,7 +36,7 @@ class TermsPrompt extends React.PureComponent {
 
   handleOK = () => {
     this.setState({ isOpen: false });
-    storageSet(CookieDecision, "yes");
+    storageSet(KEYS.COOKIE_DECISION, "yes");
     if (window.cookieDecisionCallback instanceof Function) {
       try {
         window.cookieDecisionCallback();
@@ -67,7 +48,7 @@ class TermsPrompt extends React.PureComponent {
 
   handleNo = () => {
     this.setState({ isOpen: false });
-    storageSet(CookieDecision, "no");
+    storageSet(KEYS.COOKIE_DECISION, "no");
   };
 
   renderTos() {
diff --git a/client/src/components/util/localStorage.js b/client/src/components/util/localStorage.js
new file mode 100644
index 00000000..5766d9f9
--- /dev/null
+++ b/client/src/components/util/localStorage.js
@@ -0,0 +1,22 @@
+export const KEYS = {
+  COOKIE_DECISION: "cxg.cookieDecision",
+  LOGIN_PROMPT: "cxg.LOGIN_PROMPT",
+};
+
+export function storageGet(key, defaultValue = null) {
+  try {
+    const val = window.localStorage.getItem(key);
+    if (val === null) return defaultValue;
+    return val;
+  } catch (e) {
+    return defaultValue;
+  }
+}
+
+export function storageSet(key, value) {
+  try {
+    window.localStorage.setItem(key, value);
+  } catch {
+    // continue
+  }
+}

From 86ff48ae36d55944aa3ba66b5025a5fba51c8183 Mon Sep 17 00:00:00 2001
From: maniarathi <mani.arathi@gmail.com>
Date: Fri, 9 Oct 2020 10:09:32 -0700
Subject: [PATCH 09/16] Revert "Allow columns encoded in float64 to be rendered
 as part of continuous value histograms. (#1905)" (#1925)

This reverts commit b048fd8d9a8102f479214e7b8aff85362665c88c.
---
 client/src/components/continuous/continuous.js | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/client/src/components/continuous/continuous.js b/client/src/components/continuous/continuous.js
index f1119507..ad5c908d 100644
--- a/client/src/components/continuous/continuous.js
+++ b/client/src/components/continuous/continuous.js
@@ -14,12 +14,7 @@ class Continuous extends React.PureComponent {
     if (!schema) return null;
     const obsIndex = schema.annotations.obs.index;
     const allContinuousNames = schema.annotations.obs.columns
-      .filter(
-        (col) =>
-          col.type === "int32" ||
-          col.type === "float32" ||
-          col.type === "float64"
-      )
+      .filter((col) => col.type === "int32" || col.type === "float32")
       .filter((col) => col.name !== obsIndex)
       .filter((col) => !col.writable) // skip user annotations - they will be treated as categorical
       .map((col) => col.name);

From beb46bf3df3f69eac17b527be7c3afe4d5015944 Mon Sep 17 00:00:00 2001
From: Severiano Badajoz <sbadajoz@chanzuckerberg.com>
Date: Fri, 9 Oct 2020 12:42:02 -0600
Subject: [PATCH 10/16] add and check system arg to state auth type in e2e
 test(#1924)

* add and check system arg to state auth type

* add tolower

Co-authored-by: czimergebot <35308261+czimergebot@users.noreply.github.com>
---
 client/Makefile                  | 2 +-
 client/__tests__/e2e/e2e.test.js | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/client/Makefile b/client/Makefile
index eb5c1a78..1a8d3a81 100644
--- a/client/Makefile
+++ b/client/Makefile
@@ -35,7 +35,7 @@ smoke-test:
 	start_server_and_test \
 		'CXG_OPTIONS="--config-file $(CXG_CONFIG)" $(MAKE) start-server' \
 		$(CXG_SERVER_PORT) \
-		'CXG_URL_BASE="http://localhost:$(CXG_SERVER_PORT)" npm run e2e -- --verbose false'
+		'CXG_URL_BASE="http://localhost:$(CXG_SERVER_PORT)" CXG_AUTH_TYPE="test" npm run e2e -- --verbose false'
 
 # start an instance of cellxgene and run the end-to-end annotations tests
 .PHONY: smoke-test-annotations
diff --git a/client/__tests__/e2e/e2e.test.js b/client/__tests__/e2e/e2e.test.js
index 2bd8ea8c..7e044c40 100644
--- a/client/__tests__/e2e/e2e.test.js
+++ b/client/__tests__/e2e/e2e.test.js
@@ -522,7 +522,12 @@ test("lasso moves after pan", async () => {
   expect(panCount).toBe(initialCount);
 });
 
-describe("auth buttons", () => {
+const describeIfCalledByMakeFileTarget =
+  process.env.CXG_AUTH_TYPE?.toLowerCase() === "test"
+    ? describe
+    : describe.skip;
+
+describeIfCalledByMakeFileTarget("auth buttons", () => {
   test("login then logout", async () => {
     await goToPage(appUrlBase);
     await clickOnUntil("log-in", async () => {

From 5325495123f426f0e22f4949b8eab54281b79679 Mon Sep 17 00:00:00 2001
From: maniarathi <mani.arathi@gmail.com>
Date: Mon, 12 Oct 2020 11:20:58 -0700
Subject: [PATCH 11/16] Speed up dataset drawer rendering (#1926)

---
 .../src/components/infoDrawer/infoDrawer.js   | 98 ++++---------------
 .../src/components/infoDrawer/infoFormat.js   | 88 ++++++-----------
 2 files changed, 53 insertions(+), 133 deletions(-)

diff --git a/client/src/components/infoDrawer/infoDrawer.js b/client/src/components/infoDrawer/infoDrawer.js
index 57bfd77b..9f41ef14 100644
--- a/client/src/components/infoDrawer/infoDrawer.js
+++ b/client/src/components/infoDrawer/infoDrawer.js
@@ -1,13 +1,9 @@
 import React, { PureComponent } from "react";
-import { connect, shallowEqual } from "react-redux";
+import { connect } from "react-redux";
 import { Drawer } from "@blueprintjs/core";
-import Async from "react-async";
 
 import InfoFormat from "./infoFormat";
-import {
-  selectableCategoryNames,
-  createCategorySummaryFromDfCol,
-} from "../../util/stateManager/controlsHelpers";
+import { selectableCategoryNames } from "../../util/stateManager/controlsHelpers";
 
 @connect((state) => {
   return {
@@ -20,44 +16,6 @@ import {
   };
 })
 class InfoDrawer extends PureComponent {
-  static watchAsync(props, prevProps) {
-    return !shallowEqual(props.watchProps, prevProps.watchProps);
-  }
-
-  fetchAsyncProps = async (props) => {
-    const { schema } = props.watchProps;
-    const { annoMatrix } = this.props;
-
-    const allCategoryNames = selectableCategoryNames(schema).sort();
-
-    const nonUserAnnoCategories = allCategoryNames.map((catName) => {
-      const isUserAnno = schema?.annotations?.obsByName[catName]?.writable;
-      if (!isUserAnno) return annoMatrix.fetch("obs", catName);
-      return null;
-    });
-    const singleValueCategories = (
-      await Promise.all(nonUserAnnoCategories)
-    ).reduce((acc, categoryData, i) => {
-      // Actually check to see if it is null(user anno)
-      if (!categoryData) return acc;
-      const catName = allCategoryNames[i];
-
-      const column = categoryData.icol(0);
-      const colSchema = schema.annotations.obsByName[catName];
-
-      const categorySummary = createCategorySummaryFromDfCol(column, colSchema);
-
-      const { numCategoryValues } = categorySummary;
-      //  Add to the array if the category has only one value
-      if (numCategoryValues === 1) {
-        acc.set(catName, categorySummary.allCategoryValues[0]);
-      }
-      return acc;
-    }, new Map());
-
-    return { singleValueCategories };
-  };
-
   handleClose = () => {
     const { dispatch } = this.props;
 
@@ -74,45 +32,31 @@ class InfoDrawer extends PureComponent {
       dataPortalProps,
     } = this.props;
 
+    const allCategoryNames = selectableCategoryNames(schema).sort();
+    const singleValueCategories = new Map();
+
+    allCategoryNames.forEach((catName) => {
+      const isUserAnno = schema?.annotations?.obsByName[catName]?.writable;
+      const colSchema = schema.annotations.obsByName[catName];
+      if (!isUserAnno && colSchema.categories?.length === 1) {
+        singleValueCategories.set(catName, colSchema.categories[0]);
+      }
+    });
+
     return (
       <Drawer
         title="Dataset Overview"
         onClose={this.handleClose}
         {...{ isOpen, position }}
       >
-        <Async
-          watchFn={InfoDrawer.watchAsync}
-          promiseFn={this.fetchAsyncProps}
-          watchProps={{ schema }}
-        >
-          <Async.Pending>
-            <InfoFormat
-              skeleton
-              {...{ datasetTitle, aboutURL, dataPortalProps }}
-            />
-          </Async.Pending>
-          <Async.Rejected>
-            {(error) => {
-              console.error(error);
-              return <span>Failed to load info</span>;
-            }}
-          </Async.Rejected>
-          <Async.Fulfilled>
-            {(asyncProps) => {
-              const { singleValueCategories } = asyncProps;
-              return (
-                <InfoFormat
-                  {...{
-                    datasetTitle,
-                    aboutURL,
-                    singleValueCategories,
-                    dataPortalProps,
-                  }}
-                />
-              );
-            }}
-          </Async.Fulfilled>
-        </Async>
+        <InfoFormat
+          {...{
+            datasetTitle,
+            aboutURL,
+            singleValueCategories,
+            dataPortalProps,
+          }}
+        />
       </Drawer>
     );
   }
diff --git a/client/src/components/infoDrawer/infoFormat.js b/client/src/components/infoDrawer/infoFormat.js
index eacaa406..880a5f7d 100644
--- a/client/src/components/infoDrawer/infoFormat.js
+++ b/client/src/components/infoDrawer/infoFormat.js
@@ -1,15 +1,15 @@
-import { H3, H1, UL, Classes } from "@blueprintjs/core";
+import { H3, H1, UL } from "@blueprintjs/core";
 import React from "react";
 
 import Truncate from "../util/truncate";
 
-const renderContributors = (contributors, affiliations, skeleton) => {
+const renderContributors = (contributors, affiliations) => {
   // eslint-disable-next-line no-constant-condition --  Temp removed contributor section to avoid publishing PII
   if (!contributors || contributors.length === 0 || true) return null;
   return (
     <>
-      <H3 className={skeleton ? Classes.SKELETON : null}>Contributors</H3>
-      <p className={skeleton ? Classes.SKELETON : null}>
+      <H3>Contributors</H3>
+      <p>
         {contributors.map((contributor) => {
           const { email, name, institution } = contributor;
 
@@ -22,7 +22,7 @@ const renderContributors = (contributors, affiliations, skeleton) => {
           );
         })}
       </p>
-      {renderAffiliations(affiliations, skeleton)}
+      {renderAffiliations(affiliations)}
     </>
   );
 };
@@ -39,14 +39,14 @@ const buildAffiliations = (contributors = []) => {
   return affiliations;
 };
 
-const renderAffiliations = (affiliations, skeleton) => {
+const renderAffiliations = (affiliations) => {
   if (affiliations.length === 0) return null;
   return (
     <>
-      <H3 className={skeleton ? Classes.SKELETON : null}>Affiliations</H3>
+      <H3>Affiliations</H3>
       <UL>
         {affiliations.map((item, index) => (
-          <div key={item} className={skeleton ? Classes.SKELETON : null}>
+          <div key={item}>
             <sup>{index + 1}</sup>
             {"  "}
             {item}
@@ -57,12 +57,12 @@ const renderAffiliations = (affiliations, skeleton) => {
   );
 };
 
-const renderDOILink = (type, doi, skeleton) => {
+const renderDOILink = (type, doi) => {
   if (!doi) return null;
   return (
     <>
-      <H3 className={skeleton ? Classes.SKELETON : null}>{type}</H3>
-      <p className={skeleton ? Classes.SKELETON : null}>
+      <H3>{type}</H3>
+      <p>
         <a href={doi} target="_blank" rel="noopener">
           {doi}
         </a>
@@ -71,12 +71,12 @@ const renderDOILink = (type, doi, skeleton) => {
   );
 };
 
-const renderOrganism = (organism, skeleton) => {
+const renderOrganism = (organism) => {
   if (!organism) return null;
   return (
     <>
-      <H3 className={skeleton ? Classes.SKELETON : null}>Organism</H3>
-      <p className={skeleton ? Classes.SKELETON : null}>{organism}</p>
+      <H3>Organism</H3>
+      <p>{organism}</p>
     </>
   );
 };
@@ -85,11 +85,11 @@ const ONTOLOGY_KEY = "ontology_term_id";
 const CAT_WIDTH = "30%";
 const VAL_WIDTH = "35%";
 // Render list of metadata attributes found in categorical field
-const renderSingleValueCategories = (singleValueCategories, skeleton) => {
+const renderSingleValueCategories = (singleValueCategories) => {
   if (singleValueCategories.size === 0) return null;
   return (
     <>
-      <H3 className={skeleton ? Classes.SKELETON : null}>Dataset Metadata</H3>
+      <H3>Dataset Metadata</H3>
       <UL>
         {Array.from(singleValueCategories).reduce((elems, pair) => {
           const [category, value] = pair;
@@ -115,11 +115,7 @@ const renderSingleValueCategories = (singleValueCategories, skeleton) => {
           } else {
             // Create the list item
             elems.push(
-              <li
-                className={skeleton ? Classes.SKELETON : null}
-                key={category}
-                style={{ width: "100%" }}
-              >
+              <li key={category} style={{ width: "100%" }}>
                 <Truncate>
                   <span style={{ width: CAT_WIDTH }}>{`${category}:`}</span>
                 </Truncate>
@@ -138,20 +134,17 @@ const renderSingleValueCategories = (singleValueCategories, skeleton) => {
 
 // Renders any links found in the config where link_type is not "SUMMARY"
 // If there are no links in the config, render the aboutURL
-const renderLinks = (projectLinks, aboutURL, skeleton) => {
+const renderLinks = (projectLinks, aboutURL) => {
   if (!projectLinks && !aboutURL) return null;
   if (projectLinks)
     return (
       <>
-        <H3 className={skeleton ? Classes.SKELETON : null}>Project Links</H3>
+        <H3>Project Links</H3>
         <UL>
           {projectLinks.map((link) => {
             if (link.link_type === "SUMMARY") return null;
             return (
-              <li
-                key={link.link_name}
-                className={skeleton ? Classes.SKELETON : null}
-              >
+              <li key={link.link_name}>
                 <a href={link.link_url} target="_blank" rel="noopener">
                   {link.link_name}
                 </a>
@@ -164,14 +157,9 @@ const renderLinks = (projectLinks, aboutURL, skeleton) => {
 
   return (
     <>
-      <H3 className={skeleton ? Classes.SKELETON : null}>More Info</H3>
+      <H3>More Info</H3>
       <p>
-        <a
-          className={skeleton ? Classes.SKELETON : null}
-          href={aboutURL}
-          target="_blank"
-          rel="noopener"
-        >
+        <a href={aboutURL} target="_blank" rel="noopener">
           {aboutURL}
         </a>
       </p>
@@ -179,24 +167,14 @@ const renderLinks = (projectLinks, aboutURL, skeleton) => {
   );
 };
 
-const NUM_CATEGORIES = 8;
-
-// Generates arbitrary placeholder array for singleValueCategories skeleton shape
-const singleValueCategoriesPlaceholder = Array.from(Array(NUM_CATEGORIES)).map(
-  (_, index) => {
-    return [index, index];
-  }
-);
-
 const InfoFormat = React.memo(
   ({
     datasetTitle,
-    singleValueCategories = new Map(singleValueCategoriesPlaceholder),
-    aboutURL = "thisisabouthtelengthofaurl",
+    singleValueCategories,
+    aboutURL,
     dataPortalProps = {},
-    skeleton = false,
   }) => {
-    if (dataPortalProps.corpora_schema_version === "1.0.0") {
+    if (dataPortalProps.version?.["corpora_schema_version"] !== "1.0.0") {
       dataPortalProps = {};
     }
     const {
@@ -212,15 +190,13 @@ const InfoFormat = React.memo(
 
     return (
       <div style={{ margin: 24, overflow: "auto" }}>
-        <H1 className={skeleton ? Classes.SKELETON : null}>
-          {title ?? datasetTitle}
-        </H1>
-        {renderContributors(contributors, affiliations, skeleton)}
-        {renderDOILink("DOI", doi, skeleton)}
-        {renderDOILink("Preprint DOI", preprintDOI, skeleton)}
-        {renderOrganism(organism, skeleton)}
-        {renderSingleValueCategories(singleValueCategories, skeleton)}
-        {renderLinks(projectLinks, aboutURL, skeleton)}
+        <H1>{title ?? datasetTitle}</H1>
+        {renderContributors(contributors, affiliations)}
+        {renderDOILink("DOI", doi)}
+        {renderDOILink("Preprint DOI", preprintDOI)}
+        {renderOrganism(organism)}
+        {renderSingleValueCategories(singleValueCategories)}
+        {renderLinks(projectLinks, aboutURL)}
       </div>
     );
   }

From 798976e4c1f44f35be7acc4d8a95ea2beb5b825c Mon Sep 17 00:00:00 2001
From: maniarathi <mani.arathi@gmail.com>
Date: Tue, 13 Oct 2020 15:47:56 -0700
Subject: [PATCH 12/16] Fix custom color handling (#1929)

---
 client/src/components/infoDrawer/infoFormat.js |  7 +------
 client/src/util/stateManager/colorHelpers.js   | 10 +++++++---
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/client/src/components/infoDrawer/infoFormat.js b/client/src/components/infoDrawer/infoFormat.js
index 880a5f7d..6909e7cf 100644
--- a/client/src/components/infoDrawer/infoFormat.js
+++ b/client/src/components/infoDrawer/infoFormat.js
@@ -168,12 +168,7 @@ const renderLinks = (projectLinks, aboutURL) => {
 };
 
 const InfoFormat = React.memo(
-  ({
-    datasetTitle,
-    singleValueCategories,
-    aboutURL,
-    dataPortalProps = {},
-  }) => {
+  ({ datasetTitle, singleValueCategories, aboutURL, dataPortalProps = {} }) => {
     if (dataPortalProps.version?.["corpora_schema_version"] !== "1.0.0") {
       dataPortalProps = {};
     }
diff --git a/client/src/util/stateManager/colorHelpers.js b/client/src/util/stateManager/colorHelpers.js
index 4eaea2b7..d324dd37 100644
--- a/client/src/util/stateManager/colorHelpers.js
+++ b/client/src/util/stateManager/colorHelpers.js
@@ -106,15 +106,19 @@ export function loadUserColorConfig(userColors) {
         return -1;
       })
       .reduce(
-        (acc, label, i) => {
+        (acc, label) => {
           const color = parseRGB(userColors[category][label]);
           acc[0][label] = color;
-          acc[1][i] = d3.rgb(255 * color[0], 255 * color[1], 255 * color[2]);
+          acc[1][label] = d3.rgb(
+            255 * color[0],
+            255 * color[1],
+            255 * color[2]
+          );
           return acc;
         },
         [{}, {}]
       );
-    const scale = (i) => scaleMap[i];
+    const scale = (label) => scaleMap[label];
     convertedUserColors[category] = { colors, scale };
   });
   return convertedUserColors;

From 242546371b643031d99f3be8fab3ba05f80bc1f4 Mon Sep 17 00:00:00 2001
From: Madison Dunitz <madison.dunitz@chanzuckerberg.com>
Date: Wed, 14 Oct 2020 12:46:24 -0500
Subject: [PATCH 13/16] Remove Continuous vars with 1 value from histogram, add
 to info drawer (#1927)

* remove single val continous metadata from histogram, add to info drawer

* refactor to save singleContinuous values in state

* fix edge case, single continuous values reappeard in rsb when clipped
---
 .../components/brushableHistogram/index.js    | 36 ++++++++++++++++---
 .../src/components/infoDrawer/infoDrawer.js   | 18 +++++++---
 .../src/components/infoDrawer/infoFormat.js   | 10 +++---
 client/src/reducers/index.js                  |  3 +-
 client/src/reducers/singleContinuousValue.js  | 14 ++++++++
 5 files changed, 65 insertions(+), 16 deletions(-)
 create mode 100644 client/src/reducers/singleContinuousValue.js

diff --git a/client/src/components/brushableHistogram/index.js b/client/src/components/brushableHistogram/index.js
index a18608b2..e7c6432c 100644
--- a/client/src/components/brushableHistogram/index.js
+++ b/client/src/components/brushableHistogram/index.js
@@ -450,6 +450,7 @@ const Histogram = ({
     isScatterplotYYaccessor: state.controls.scatterplotYYaccessor === field,
     continuousSelectionRange: state.continuousSelection[myName],
     isColorAccessor: state.colors.colorAccessor === field,
+    singleContinuousValues: state.singleContinuousValue.singleContinuousValues,
   };
 })
 class HistogramBrush extends React.PureComponent {
@@ -608,18 +609,44 @@ class HistogramBrush extends React.PureComponent {
   };
 
   fetchAsyncProps = async () => {
-    const { annoMatrix } = this.props;
+    const { annoMatrix, field, dispatch, singleContinuousValues } = this.props;
     const { isClipped } = annoMatrix;
-
+    if (singleContinuousValues.has(field)) {
+      return {
+        histogram: undefined,
+        range: undefined,
+        unclippedRange: undefined,
+        unclippedRangeColor: globals.blue,
+        isSingleValue: true,
+        OK2Render: false,
+      };
+    }
     const query = this.createQuery();
     const df = await annoMatrix.fetch(...query);
     const column = df.icol(0);
 
-    // if we are clipped, fetch both our value and our unclipped value,
-    // as we need the absolute min/max range, not just the clipped min/max.
     const summary = column.summarize();
     const range = [summary.min, summary.max];
 
+    if (summary.min === summary.max && !isClipped) {
+      dispatch({
+        type: "add single continuous value",
+        field,
+        value: summary.min,
+      });
+      return {
+        histogram: undefined,
+        range,
+        unclippedRange: range,
+        unclippedRangeColor: globals.blue,
+        isSingleValue: true,
+        OK2Render: false,
+      };
+    }
+
+    const isSingleValue = summary.min === summary.max;
+    // if we are clipped, fetch both our value and our unclipped value,
+    // as we need the absolute min/max range, not just the clipped min/max.
     let unclippedRange = [...range];
     if (isClipped) {
       const parent = await annoMatrix.viewOf.fetch(...query);
@@ -643,7 +670,6 @@ class HistogramBrush extends React.PureComponent {
       this.height
     );
 
-    const isSingleValue = summary.min === summary.max;
     const nonFiniteExtent =
       summary.min === undefined ||
       summary.max === undefined ||
diff --git a/client/src/components/infoDrawer/infoDrawer.js b/client/src/components/infoDrawer/infoDrawer.js
index 9f41ef14..aad395bc 100644
--- a/client/src/components/infoDrawer/infoDrawer.js
+++ b/client/src/components/infoDrawer/infoDrawer.js
@@ -1,7 +1,6 @@
 import React, { PureComponent } from "react";
-import { connect } from "react-redux";
+import { connect, shallowEqual } from "react-redux";
 import { Drawer } from "@blueprintjs/core";
-
 import InfoFormat from "./infoFormat";
 import { selectableCategoryNames } from "../../util/stateManager/controlsHelpers";
 
@@ -13,9 +12,14 @@ import { selectableCategoryNames } from "../../util/stateManager/controlsHelpers
     aboutURL: state.config?.links?.["about-dataset"],
     isOpen: state.controls.datasetDrawer,
     dataPortalProps: state.config?.["corpora_props"] ?? {},
+    singleContinuousValues: state.singleContinuousValue.singleContinuousValues,
   };
 })
 class InfoDrawer extends PureComponent {
+  static watchAsync(props, prevProps) {
+    return !shallowEqual(props.watchProps, prevProps.watchProps);
+  }
+
   handleClose = () => {
     const { dispatch } = this.props;
 
@@ -30,18 +34,22 @@ class InfoDrawer extends PureComponent {
       schema,
       isOpen,
       dataPortalProps,
+      singleContinuousValues,
     } = this.props;
 
     const allCategoryNames = selectableCategoryNames(schema).sort();
-    const singleValueCategories = new Map();
+    const allSingleValues = new Map();
 
     allCategoryNames.forEach((catName) => {
       const isUserAnno = schema?.annotations?.obsByName[catName]?.writable;
       const colSchema = schema.annotations.obsByName[catName];
       if (!isUserAnno && colSchema.categories?.length === 1) {
-        singleValueCategories.set(catName, colSchema.categories[0]);
+        allSingleValues.set(catName, colSchema.categories[0]);
       }
     });
+    singleContinuousValues.forEach((value, catName) => {
+      allSingleValues.set(catName, value);
+    });
 
     return (
       <Drawer
@@ -53,7 +61,7 @@ class InfoDrawer extends PureComponent {
           {...{
             datasetTitle,
             aboutURL,
-            singleValueCategories,
+            allSingleValues,
             dataPortalProps,
           }}
         />
diff --git a/client/src/components/infoDrawer/infoFormat.js b/client/src/components/infoDrawer/infoFormat.js
index 6909e7cf..04de063b 100644
--- a/client/src/components/infoDrawer/infoFormat.js
+++ b/client/src/components/infoDrawer/infoFormat.js
@@ -85,13 +85,13 @@ const ONTOLOGY_KEY = "ontology_term_id";
 const CAT_WIDTH = "30%";
 const VAL_WIDTH = "35%";
 // Render list of metadata attributes found in categorical field
-const renderSingleValueCategories = (singleValueCategories) => {
-  if (singleValueCategories.size === 0) return null;
+const renderSingleValues = (singleValues) => {
+  if (singleValues.size === 0) return null;
   return (
     <>
       <H3>Dataset Metadata</H3>
       <UL>
-        {Array.from(singleValueCategories).reduce((elems, pair) => {
+        {Array.from(singleValues).reduce((elems, pair) => {
           const [category, value] = pair;
           // If the value is empty skip it
           if (!value) return elems;
@@ -168,7 +168,7 @@ const renderLinks = (projectLinks, aboutURL) => {
 };
 
 const InfoFormat = React.memo(
-  ({ datasetTitle, singleValueCategories, aboutURL, dataPortalProps = {} }) => {
+  ({ datasetTitle, allSingleValues, aboutURL, dataPortalProps = {} }) => {
     if (dataPortalProps.version?.["corpora_schema_version"] !== "1.0.0") {
       dataPortalProps = {};
     }
@@ -190,7 +190,7 @@ const InfoFormat = React.memo(
         {renderDOILink("DOI", doi)}
         {renderDOILink("Preprint DOI", preprintDOI)}
         {renderOrganism(organism)}
-        {renderSingleValueCategories(singleValueCategories)}
+        {renderSingleValues(allSingleValues)}
         {renderLinks(projectLinks, aboutURL)}
       </div>
     );
diff --git a/client/src/reducers/index.js b/client/src/reducers/index.js
index f40dd946..5305706b 100644
--- a/client/src/reducers/index.js
+++ b/client/src/reducers/index.js
@@ -21,7 +21,7 @@ import centroidLabels from "./centroidLabels";
 import pointDialation from "./pointDilation";
 import { reembedController } from "./reembed";
 import { gcMiddleware as annoMatrixGC } from "../annoMatrix";
-
+import singleContinuousValue from "./singleContinuousValue";
 import undoableConfig from "./undoableConfig";
 
 const Reducer = undoable(
@@ -32,6 +32,7 @@ const Reducer = undoable(
     ["ontology", ontology],
     ["annotations", annotations],
     ["layoutChoice", layoutChoice],
+    ["singleContinuousValue", singleContinuousValue],
     ["categoricalSelection", categoricalSelection],
     ["continuousSelection", continuousSelection],
     ["graphSelection", graphSelection],
diff --git a/client/src/reducers/singleContinuousValue.js b/client/src/reducers/singleContinuousValue.js
new file mode 100644
index 00000000..7bea9dfe
--- /dev/null
+++ b/client/src/reducers/singleContinuousValue.js
@@ -0,0 +1,14 @@
+const initialState = {
+  singleContinuousValues: new Map(),
+};
+const singleContinuousValue = (state = initialState, action) => {
+  switch (action.type) {
+    case "add single continuous value":
+      state.singleContinuousValues.set(action.field, action.value);
+      return state;
+    default:
+      return state;
+  }
+};
+
+export default singleContinuousValue;

From c9f95491182d134332edf8fe218b8ff2554ecff0 Mon Sep 17 00:00:00 2001
From: Severiano Badajoz <sbadajoz@chanzuckerberg.com>
Date: Fri, 16 Oct 2020 11:59:28 -0700
Subject: [PATCH 14/16] Adopt JS standards once `userinfo` data is in frontend
 (#1930)

---
 client/src/actions/index.js                      |  8 ++++----
 client/src/components/autosave/filenameDialog.js |  6 +++---
 client/src/components/categorical/index.js       |  8 ++++----
 client/src/components/menubar/authButtons.js     | 12 ++++++------
 client/src/components/menubar/index.js           |  6 +++---
 client/src/reducers/index.js                     |  4 ++--
 client/src/reducers/{userinfo.js => userInfo.js} |  5 ++---
 7 files changed, 24 insertions(+), 25 deletions(-)
 rename client/src/reducers/{userinfo.js => userInfo.js} (83%)

diff --git a/client/src/actions/index.js b/client/src/actions/index.js
index 8730730e..a71149ee 100644
--- a/client/src/actions/index.js
+++ b/client/src/actions/index.js
@@ -43,12 +43,12 @@ async function configFetch(dispatch) {
 
 async function userInfoFetch(dispatch) {
   return fetchJson("userinfo").then((response) => {
-    const { userinfo } = response || {};
+    const { userinfo: userInfo } = response || {};
     dispatch({
-      type: "userinfo load complete",
-      userinfo,
+      type: "userInfo load complete",
+      userInfo,
     });
-    return userinfo;
+    return userInfo;
   });
 }
 
diff --git a/client/src/components/autosave/filenameDialog.js b/client/src/components/autosave/filenameDialog.js
index 08fe1462..3ff7e500 100644
--- a/client/src/components/autosave/filenameDialog.js
+++ b/client/src/components/autosave/filenameDialog.js
@@ -15,7 +15,7 @@ import {
   idhash: state.config?.parameters?.["annotations-user-data-idhash"] ?? null,
   annotations: state.annotations,
   auth: state.config?.authentication,
-  userinfo: state.userinfo,
+  userInfo: state.userInfo,
   writableCategoriesEnabled: state.config?.parameters?.annotations ?? false,
 }))
 class FilenameDialog extends React.Component {
@@ -97,7 +97,7 @@ class FilenameDialog extends React.Component {
       writableCategoriesEnabled,
       annotations,
       idhash,
-      userinfo,
+      userInfo,
     } = this.props;
     const { filenameText } = this.state;
 
@@ -105,7 +105,7 @@ class FilenameDialog extends React.Component {
       annotations.promptForFilename &&
       !annotations.dataCollectionNameIsReadOnly &&
       !annotations.dataCollectionName &&
-      userinfo.is_authenticated ? (
+      userInfo.is_authenticated ? (
       <Dialog
         icon="tag"
         title="Annotations Collection"
diff --git a/client/src/components/categorical/index.js b/client/src/components/categorical/index.js
index 63db6cac..9a267930 100644
--- a/client/src/components/categorical/index.js
+++ b/client/src/components/categorical/index.js
@@ -15,7 +15,7 @@ import actions from "../../actions";
   writableCategoriesEnabled: state.config?.parameters?.annotations ?? false,
   schema: state.annoMatrix?.schema,
   ontology: state.ontology,
-  userinfo: state.userinfo,
+  userInfo: state.userInfo,
 }))
 class Categories extends React.Component {
   constructor(props) {
@@ -132,7 +132,7 @@ class Categories extends React.Component {
       writableCategoriesEnabled,
       schema,
       ontology,
-      userinfo,
+      userInfo,
     } = this.props;
     const ontologyEnabled = ontology?.enabled ?? false;
     /* all names, sorted in display order.  Will be rendered in this order */
@@ -213,7 +213,7 @@ class Categories extends React.Component {
         {writableCategoriesEnabled ? (
           <Tooltip
             content={
-              userinfo.is_authenticated
+              userInfo.is_authenticated
                 ? "Create a new category"
                 : "You must be logged in to create new categorical fields"
             }
@@ -230,7 +230,7 @@ class Categories extends React.Component {
               data-testid="open-annotation-dialog"
               onClick={this.handleEnableAnnoMode}
               intent="primary"
-              disabled={!userinfo.is_authenticated}
+              disabled={!userInfo.is_authenticated}
             >
               Create new category
             </AnchorButton>
diff --git a/client/src/components/menubar/authButtons.js b/client/src/components/menubar/authButtons.js
index 91729679..91af0b42 100644
--- a/client/src/components/menubar/authButtons.js
+++ b/client/src/components/menubar/authButtons.js
@@ -31,11 +31,11 @@ const LOGIN_PROMPT_OFF = "off";
 const Auth = React.memo((props) => {
   const [isPromptOpen, setIsPromptOpen] = useState(shouldShowPrompt());
 
-  const { auth, userinfo } = props;
+  const { auth, userInfo } = props;
 
-  const isAuthenticated = userinfo && userinfo.is_authenticated;
+  const isAuthenticated = userInfo && userInfo.is_authenticated;
 
-  window.userinfo = userinfo;
+  window.userInfo = userInfo;
 
   const randomInt = Math.random() * 15;
   const sexIndex = Math.floor(randomInt / 5);
@@ -55,7 +55,7 @@ const Auth = React.memo((props) => {
       <Menu>
         <MenuItem
           data-testid="user-email"
-          text={`Logged in as: ${userinfo.email}`}
+          text={`Logged in as: ${userInfo.email}`}
         />
         <MenuItem
           data-testid="log-out"
@@ -74,8 +74,8 @@ const Auth = React.memo((props) => {
           style={{ padding: 0 }}
         >
           {/*  eslint-disable-next-line no-constant-condition -- disable profile picture until CSP is tweaked */}
-          {userinfo?.picture && false ? (
-            <img alt="profile" size="21px" src={userinfo?.picture} />
+          {userInfo?.picture && false ? (
+            <img alt="profile" size="21px" src={userInfo?.picture} />
           ) : (
             <span style={{ fontSize: "18px" }}>{scientist}</span>
           )}
diff --git a/client/src/components/menubar/index.js b/client/src/components/menubar/index.js
index 115eb6ff..cbfeec5e 100644
--- a/client/src/components/menubar/index.js
+++ b/client/src/components/menubar/index.js
@@ -42,7 +42,7 @@ import { getEmbSubsetView } from "../../util/stateManager/viewStackHelpers";
     celllist2: state.differential.celllist2,
     libraryVersions: state.config?.["library_versions"],
     auth: state.config?.authentication,
-    userinfo: state.userinfo,
+    userInfo: state.userInfo,
     undoDisabled: state["@@undoable/past"].length === 0,
     redoDisabled: state["@@undoable/future"].length === 0,
     aboutLink: state.config?.links?.["about-dataset"],
@@ -217,7 +217,7 @@ class MenuBar extends React.PureComponent {
       subsetPossible,
       subsetResetPossible,
       enableReembedding,
-      userinfo,
+      userInfo,
       auth,
     } = this.props;
     const { pendingClipPercentiles } = this.state;
@@ -244,7 +244,7 @@ class MenuBar extends React.PureComponent {
           zIndex: 3,
         }}
       >
-        <AuthButtons auth={auth} userinfo={userinfo} />
+        <AuthButtons {...{ auth, userInfo }} />
         <UndoRedoReset
           dispatch={dispatch}
           undoDisabled={undoDisabled}
diff --git a/client/src/reducers/index.js b/client/src/reducers/index.js
index 5305706b..b4a47153 100644
--- a/client/src/reducers/index.js
+++ b/client/src/reducers/index.js
@@ -4,7 +4,7 @@ import thunk from "redux-thunk";
 import cascadeReducers from "./cascade";
 import undoable from "./undoable";
 import config from "./config";
-import userinfo from "./userinfo";
+import userInfo from "./userInfo";
 import annoMatrix from "./annoMatrix";
 import obsCrossfilter from "./obsCrossfilter";
 import categoricalSelection from "./categoricalSelection";
@@ -43,7 +43,7 @@ const Reducer = undoable(
     ["pointDilation", pointDialation],
     ["reembedController", reembedController],
     ["autosave", autosave],
-    ["userinfo", userinfo],
+    ["userInfo", userInfo],
   ]),
   [
     "annoMatrix",
diff --git a/client/src/reducers/userinfo.js b/client/src/reducers/userInfo.js
similarity index 83%
rename from client/src/reducers/userinfo.js
rename to client/src/reducers/userInfo.js
index 4765c822..6939e949 100644
--- a/client/src/reducers/userinfo.js
+++ b/client/src/reducers/userInfo.js
@@ -1,4 +1,3 @@
-// jshint esversion: 6
 const UserInfo = (state = {}, action) => {
   switch (action.type) {
     case "initial data load start":
@@ -7,12 +6,12 @@ const UserInfo = (state = {}, action) => {
         loading: true,
         error: null,
       };
-    case "userinfo load complete":
+    case "userInfo load complete":
       return {
         ...state,
         loading: false,
         error: null,
-        ...action.userinfo,
+        ...action.userInfo,
       };
     case "initial data load error":
       return {

From 6a741956e1ee92496c09943f5f8fd8d19f1462a8 Mon Sep 17 00:00:00 2001
From: bmccandless <bmccandless@chanzuckerberg.com>
Date: Fri, 16 Oct 2020 14:02:05 -0700
Subject: [PATCH 15/16] Update readme for eb server. (#1928)

* Update readme for eb server.

Update the README with new way of handling secrets.
Update portions that were out of date.
Add a section for Authentication and a placeholder for User Annotations.

Also remove an obsolete function that processes the AWS secrets.

  #1522

Co-authored-by: Madison Dunitz <madison.dunitz@chanzuckerberg.com>
---
 server/common/config/__init__.py              |  64 +------
 server/eb/README.md                           | 165 +++++++++++++-----
 server/eb/app.py                              |  20 +--
 server/eb/check_config.py                     |   2 +-
 .../unit/common/config/test_server_config.py  |  27 ---
 5 files changed, 123 insertions(+), 155 deletions(-)

diff --git a/server/common/config/__init__.py b/server/common/config/__init__.py
index fb2439e7..8a74427b 100644
--- a/server/common/config/__init__.py
+++ b/server/common/config/__init__.py
@@ -1,66 +1,4 @@
-import logging
-import os
-import sys
-
-from server.common.aws_secret_utils import get_secret_key
-from server.common.data_locator import discover_s3_region_name
+from server.common.aws_secret_utils import get_secret_key  # noqa F504
 
 DEFAULT_SERVER_PORT = 5005
 BIG_FILE_SIZE_THRESHOLD = 100 * 2 ** 20  # 100MB
-
-
-def handle_config_from_secret(app_config):
-    """Update configuration from the secret manager"""
-    secret_name = os.getenv("CXG_AWS_SECRET_NAME")
-    if not secret_name:
-        return
-
-    # need to find the secret manager region.
-    #  1. from CXG_AWS_SECRET_REGION_NAME
-    #  2. discover from dataroot location (if on s3)
-    #  3. discover from config file location (if on s3)
-    secret_region_name = os.getenv("CXG_AWS_SECRET_REGION_NAME")
-    if secret_region_name is None:
-        secret_region_name = discover_s3_region_name(app_config.multi_dataset__dataroot)
-        if not secret_region_name:
-            from server.eb.app import config_file
-
-            secret_region_name = discover_s3_region_name(config_file)
-    if not secret_region_name:
-        logging.error("Could not determine the AWS Secret Manager region")
-        sys.exit(1)
-
-    secrets = get_secret_key(secret_region_name, secret_name)
-
-    if not secrets:
-        return
-
-    server_attrs = (
-        ("flask_secret_key", "app__flask_secret_key"),
-        ("oauth_client_secret", "authentication__params_oauth__client_secret"),
-    )
-    default_dataset_attrs = (("db_uri", "user_annotations__hosted_tiledb_array__db_uri"),)
-
-    # update server configuration attributes
-    for key, attr in server_attrs:
-        cur_val = getattr(app_config.server_config, attr)
-        if cur_val:
-            continue
-
-        # replace the attr with the secret if it is not set
-        val = secrets.get(key)
-        if val:
-            logging.info(f"set {attr} from secret")
-            app_config.update_server_config(**{attr: val})
-
-    # update default dataset configuration attributes
-    for key, attr in default_dataset_attrs:
-        cur_val = getattr(app_config.default_dataset_config, attr)
-        if cur_val:
-            continue
-
-        # replace the attr with the secret if it is not set
-        val = secrets.get(key)
-        if val:
-            logging.info(f"set {attr} from secret")
-            app_config.update_default_dataset_config(**{attr: val})
diff --git a/server/eb/README.md b/server/eb/README.md
index f4209f15..306eda00 100644
--- a/server/eb/README.md
+++ b/server/eb/README.md
@@ -1,12 +1,12 @@
 # AWS Elastic Beanstalk
 
-This directory contains script to aid in creating and deploying cellxgene on
-an AWS Elastic Beanstalk instance.
+This directory contains scripts to aid in creating and deploying cellxgene on
+AWS Elastic Beanstalk.
 
 This will result in a variant of cellxgene, running on AWS EC2 instances, serving data from S3.
-All datasets must be in the new CXG (tiledb) format - see the converter script cxgtool.py
-in server/converters - and located in a single S3 prefix, which is accessible to the instance.
-In the current incarnation, no access control or authentication support is available
+All datasets must be in the CXG (tiledb) format (see `cellxene convert --help`),
+and located under a single S3 prefix, which is accessible to the instance.
+In the current incarnation, no access control is available
 (outside of anything you configure yourself), so this is most appropriate for public datasets.
 
 This is early development work, and will change significantly in the near future.
@@ -17,10 +17,10 @@ We would love feedback on it, but please assume it will change.
 1. Some familiarity with AWS EB, S3, and IAM are needed.
 
 2. Install the awsebcli.
-Instruction are here:
-https://docs.aws.amazon.com/elasticbeanstalk/latest/dg/eb-cli3-install.html
+   Instruction are here:
+   https://docs.aws.amazon.com/elasticbeanstalk/latest/dg/eb-cli3-install.html
 
-3. In the top level directory, run ```make build-client``` to create the client static assets.
+3. In the top level directory, run `make build-client` to create the client static assets.
 
 ## Steps
 
@@ -31,20 +31,21 @@ There are many more options to these commands that may be important or necessary
 
 The following choices are known to work.
 
-* S3 Bucket.
-* POSIX filesystem (such as Lustre)
-* Lustre filesystem backed by S3
+- S3 Bucket.
+- POSIX filesystem (such as Lustre)
+- Lustre filesystem backed by S3
 
 S3 is convenient and the relatively inexpensive option.
 Lustre is higher performance, but more expensive, and slightly more complex to setup and manage.
-AWS supports a feature to back the Lustre filesystem with S3, which give an easy to manage and high
+AWS supports a feature to back the Lustre filesystem with S3, which gives an easy to manage, high
 performance option.
 
-Once the storage is in place, the next step is to copy your matrix files to that location.
-Currently cellxgene supports a flat file organization.   Each matrix file is located from
-the same s3 prefix or filesystem directory.  This location is specified in the configuration as the dataroot.
+Once the storage is in place, the next step is to copy your data files to that location.
+Currently cellxgene supports a flat file organization. Each matrix file is located under
+the same s3 prefix or filesystem directory. This location is specified in the configuration
+as the dataroot.
 
-### 2. Create an elastic beanstalk application.  For example:
+### 2. Create an elastic beanstalk application. For example:
 
 ```
 EB_APP=cellxgene-app
@@ -54,9 +55,9 @@ eb init -p python-3.6 $EB_APP
 ### 3. Configuring cellxgene
 
 All the cellxgene configuration options can be set from a configuration file.
-This file can be generated like this:
+A yaml config file containing all of the default configuration options can be generated like this:
 
-```cellxgene launch --dump-default-config > myconfig.yaml```
+`cellxgene launch --dump-default-config > myconfig.yaml`
 
 The config file may then be customized before the app is deployed.
 
@@ -66,18 +67,14 @@ First, if your config file is named "config.yaml" and exists in `customize/confi
 then it will be bundled with the application zip file and installed along
 side the app on the EB servers.
 
-Second, a potentially more flexible approach is to place your config file in a location accessible to the EB
-servers, such as in S3.  For example:  s3://my-bucket/my-datasets/config.yaml.
+Second, a potentially more flexible approach is to place your config file in a location accessible
+to the EB servers, such as in S3. For example: s3://my-bucket/my-datasets/config.yaml.
 Set the CXG_CONFIG_FILE environment variable to specify this location.
 
-Another option is to set the CXG_DATAROOT environment variable.  The dataroot
+Another option is to set the CXG_DATAROOT environment variable. The dataroot
 is the location where the matrix files are located.
 This environment variable will override the dataroot in the config file (if specified).
 
-- Note:  Certain features, such as user annotations, are automatically disabled by the EB app,
-and cannot be enabled using configuration.  They may be enabled manually by modifying app.py, however
-this is not supported or recommended at this time.
-
 ### 4. Customization
 
 The deployment can be customized in several ways, by adding files to a directory called
@@ -93,15 +90,15 @@ The cellxgene server can serve additional static webpages that will be associate
 These include the about_legal_tos (terms of service), and about_legal_privacy, for example.
 To use this feature, do the following:
 
-* In this directory, create a sub directory called "customize/deploy/".
-* Copy the files you want to serve into this directory
-* modify your configuration file to set the location to these file:  /static/cellxgene/deploy/<filename>
+- In this directory, create a sub directory called "customize/deploy/".
+- Copy the files you want to serve into this directory
+- modify your configuration file to set the location to these file: /static/cellxgene/deploy/<filename>
 
-Example:  you want to include an "about_legal_tos" and "about_legal_privacy" page to cellxgene.
+Example: you want to include an "about_legal_tos" and "about_legal_privacy" page to cellxgene.
 Assume files called "tos.html" and "privacy.html" exist.
 
 ```
-$ mkdir static
+$ mkdir -p customize/deploy
 $ cp <source_dir>/tos.html customize/deploy/tos.html
 $ cp <source_dir>/privacy.html customize/deploy/privacy.html
 
@@ -116,14 +113,14 @@ about_legal_privacy: /static/cellxgene/deploy/privacy.html
 Additional scripts can be added using the server/inline_scripts config parameters.
 To include these scripts in the deployment, use the following steps:
 
-* In this directory, create a sub directory called "customize/inline_scripts".
-* Copy the script files into this directory
-* modify your configuration file to set the location to these file (leaving off customize/inline_scripts)
+- In this directory, create a sub directory called "customize/inline_scripts".
+- Copy the script files into this directory
+- Modify your configuration file to set the location to these file (leaving off customize/inline_scripts)
 
 For example, to add an inline script called "myscript.js":
 
 ```
-$ mkdir scripts
+$ mkdir -p customize/inline_scripts
 $ cp <source_dir>/myscript.js customize/inline_scripts/myscript.js
 # edit the config.yaml
 $ grep inline_scripts config.yaml
@@ -135,14 +132,14 @@ $ grep inline_scripts config.yaml
 Optionally, you can add plugins to the server python code. To include a plugin in the deployment use the following steps:
 
 ```
-$ mkdir plugins
+$ mkdir -p customize/plugins
 $ cp <source_dir>/<my_plugin>.py customize/plugins/<my_plugin>.py
 ```
 
 #### ebextensions
 
 Any additional config files intended for the `.ebextensions` directory of the artifact can be added
-to the `customize/ebextensions` directory.  Any file found here will be copied over.
+to the `customize/ebextensions` directory. Any file found here will be copied over.
 
 #### requirements.txt
 
@@ -152,6 +149,7 @@ This is useful to ensure that the dependencies do not change from one deployment
 Therefore the custom/requirements.txt must all have exact versions specified (e.g. anndata==0.7.1).
 
 This file can be generated the first time using a process like this:
+
 ```
 #  assume you are running in this directory
 $  virtualenv temp
@@ -168,6 +166,20 @@ If a future cellxgene version updates its requirements by modifying a module ver
 or adding a new dependency, then the `make build` process will detect any
 incompatibilities and raise an error.
 
+#### File structure for customizations
+
+The following diagram shows the file structure for the customization directory.
+
+```
+customization
++-- config.yaml
++-- deploy/  
++-- inline_scripts/
++-- plugins/
++-- ebextensions/ 
++-- requirements.txt
+```
+
 ### 5. Create the artifact.zip file for the application
 
 ```
@@ -176,19 +188,13 @@ $ make build
 
 ### 6. Flask secret key
 
-The application requires as secret key to be provided to flask, the web framework used by cellxgene.
+The application requires a secret key to be provided to flask, the web framework used by cellxgene.
 There are three ways to provide the secret key:
 
-- In the configuration file: update the server/flask_secret_key attribute.
+- In the configuration file, update the server/flask_secret_key attribute.
+- In the configuration file, update the external/aws_secrets_manager section to set the
+  secret name and key that defines the flask secret key.
 - An environment variable: `CXG_SECRET_KEY`
-- Managed by the AWS Secret Manager
-
-If using the AWS Secret Manager, then the secret name is passed as an environment variable:  CXG_AWS_SECRET_NAME.
-The secret must contain a key with the name "flask_secret_key".
-The region name for the AWS Secret Manager must be specified (e.g. us-east-1).
-The most straightforward way is to specified it with the CXG_AWS_SECRET_REGION_NAME environment variable.
-If this environment variable is not defined, then the app attempts to determine the region from the
-dataroot (if in s3), or the config file location (if in s3).
 
 ### 7. Create an environment
 
@@ -203,7 +209,8 @@ $ EB_INSTANCE=m5.large
 $ CXG_DATAROOT=<location to your S3 bucket>
 $ CXG_CONFIG_FILE=<location to your config file>
 
-# Potentially also set envvars for the secret key.
+# Potentially also set an environment variable for the flask secret key,
+# and other environemet variable described in the configuration file.
 
 $ eb create $EB_ENV --instance-type $EB_INSTANCE \
    --envvars CXG_DATAROOT=$CXG_DATAROOT,CXG_CONFIG_FILE=$CXG_CONFIG_FILE
@@ -227,3 +234,67 @@ $ eb deploy $EB_ENV
 ```
 $ eb open $EB_ENV
 ```
+
+## Advanced Features
+
+### Authentication
+
+Authentication can be configured in the configuration file. Authentication is required
+for User Annotations (see below). User Annotations is a feature where annotations can be
+created by the user
+, and
+then associated with the user's id.
+When the user revisits the site, their annotations will be available.
+
+There are three main authentication modes: null, session, or oauth.  
+In the configuration file specify the authentication mode by setting
+`server / authentication / type`.
+
+#### null
+
+Authentication is disabled: user annotations cannot be enabled.
+
+#### session
+
+The user is associated with their client browser session. This approach is
+simple to setup, but not recommended for hosted cellxgene, since the user will not have access to
+their annotations when running from a different browser, or if their cookies get cleared.
+
+#### oauth
+
+A user logs into cellxgene using an identity provider (like Google), or logs in using
+an email/password. This is the best option, but requires making use of an oauth service and
+additional configuration of the cellxgene server.
+
+To see what this looks like, please look at https://cellxgene.cziscience.com/,
+and view one of the cellxgene datasets.
+For this server, Auth0 (auth0.com) is used for authentication, but there are other options.
+There are good sources of documentation online that describe how to use one of these
+services.
+
+The `params_oauth` section in the configuration file describes characteristics of the
+authentication service, like "client_id" and "client_secret".  
+For security, the client_secret needs to be protected. One option is to
+store it in the AWS Secrets Manager.
+
+### User Annotations
+
+User annotations can be configured in the configuration file both generally and for a specific data route. The annotations feature is only available when Authorization is enabled. 
+To enable Annotations, it is necessary to create a relational database and add the database uri (typically `postgresql://[user[:password]@][netloc][:port][/dbname]`) to the secrets manager under `DB_URI`. 
+The hosted version of cellxgene runs on AWS's [Aurora PostgreSQL](https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/Aurora.AuroraPostgreSQL.html) but any sqlalchemy compatible relational database should work. 
+Once the database is set up apply the cellxgene schema to your database by running the following inside the cellxgene repo
+`PROJECT_ROOT=$(git rev-parse --show-toplevel)`
+`python3`
+Inside the python console
+`from sqlalchemy import create_engine`
+`from server.db.cellxgene_orm import Base`
+`uri = "[DB_URI]”`
+`engine = create_engine(uri)`
+
+ Base.metadata.create_all(engine)`
+
+To check the schema was properly applied (or just to check what is in the database at any point)
+ssh into your database. For a postgres database this entails running:  
+`psql [DB_URI]`
+
+You'll also need to update your IAM policies to allow the instance to write to the s3 bucket.
diff --git a/server/eb/app.py b/server/eb/app.py
index 0844f599..4b2212f5 100644
--- a/server/eb/app.py
+++ b/server/eb/app.py
@@ -9,8 +9,6 @@ from flask import json
 import logging
 from flask_talisman import Talisman
 from flask_cors import CORS
-from server.common.config import handle_config_from_secret
-from server.common.errors import SecretKeyRetrievalError
 
 
 if os.path.isdir("/opt/python/log"):
@@ -165,26 +163,14 @@ try:
         logging.info("Configuration from CXG_DATAROOT")
         app_config.update_server_config(multi_dataset__dataroot=dataroot)
 
-    # update from secret manager
-    try:
-        handle_config_from_secret(app_config)
-    except SecretKeyRetrievalError:
-        sys.exit(1)
-
-    # features are unsupported in the current hosted server
+    # overwrite configuration for the eb app
     app_config.update_default_dataset_config(embeddings__enable_reembedding=False,)
     app_config.update_server_config(multi_dataset__allowed_matrix_types=["cxg"],)
+
+    # complete config
     app_config.complete_config(logging.info)
 
-    if not app_config.server_config.app__flask_secret_key:
-        logging.critical(
-            "flask_secret_key is not provided.  Either set in config file, CXG_SECRET_KEY environment variable, "
-            "or in AWS Secret Manager"
-        )
-        sys.exit(1)
-
     server = WSGIServer(app_config)
-
     debug = False
     application = server.app
 
diff --git a/server/eb/check_config.py b/server/eb/check_config.py
index 6886e101..2ea1c25e 100644
--- a/server/eb/check_config.py
+++ b/server/eb/check_config.py
@@ -19,8 +19,8 @@ def main():
     args = parser.parse_args()
 
     app_config = AppConfig()
-    app_config.update_from_config_file(args.config_file)
     try:
+        app_config.update_from_config_file(args.config_file)
         app_config.complete_config()
     except Exception as e:
         print(f"Error: {str(e)}")
diff --git a/server/test/unit/common/config/test_server_config.py b/server/test/unit/common/config/test_server_config.py
index f862a40f..69257bfd 100644
--- a/server/test/unit/common/config/test_server_config.py
+++ b/server/test/unit/common/config/test_server_config.py
@@ -310,30 +310,3 @@ class TestServerConfig(ConfigTests):
         mock_tiledb_context.assert_called_once_with(
             {"sm.tile_cache_size": 10, "sm.num_reader_threads": 2, "vfs.s3.region": "us-east-1"}
         )
-
-    @mockenv(CXG_AWS_SECRET_NAME="TESTING", CXG_AWS_SECRET_REGION_NAME="TEST_REGION")
-    @patch("server.common.config.get_secret_key")
-    def test_get_config_vars_from_aws_secrets(self, mock_get_secret_key):
-        mock_get_secret_key.return_value = {
-            "flask_secret_key": "mock_flask_secret",
-            "oauth_client_secret": "mock_oauth_secret",
-            "db_uri": "mock_db_uri",
-        }
-
-        config = AppConfig()
-
-        with self.assertLogs(level="INFO") as logger:
-            from server.common.config import handle_config_from_secret
-
-            # should not throw error
-            # "AttributeError: 'XConfig' object has no attribute 'x'"
-            handle_config_from_secret(config)
-
-            # should log 3 lines (one for each var set from a secret)
-            self.assertEqual(len(logger.output), 3)
-            self.assertIn("INFO:root:set app__flask_secret_key from secret", logger.output[0])
-            self.assertIn("INFO:root:set authentication__params_oauth__client_secret from secret", logger.output[1])
-            self.assertIn("INFO:root:set user_annotations__hosted_tiledb_array__db_uri from secret", logger.output[2])
-            self.assertEqual(config.server_config.app__flask_secret_key, "mock_flask_secret")
-            self.assertEqual(config.server_config.authentication__params_oauth__client_secret, "mock_oauth_secret")
-            self.assertEqual(config.default_dataset_config.user_annotations__hosted_tiledb_array__db_uri, "mock_db_uri")

From 377e4bccaa51d12142b1e643c8fdc14ff94acc31 Mon Sep 17 00:00:00 2001
From: maniarathi <mani.arathi@gmail.com>
Date: Mon, 19 Oct 2020 10:31:36 -0700
Subject: [PATCH 16/16] Remove errornous checking for converting float64 to
 float32. In reality the slight difference by downcasting is totally fine.
 (#1935)

---
 server/common/utils/type_conversion_utils.py | 23 ++++++--------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/server/common/utils/type_conversion_utils.py b/server/common/utils/type_conversion_utils.py
index ccda1177..762dccb2 100644
--- a/server/common/utils/type_conversion_utils.py
+++ b/server/common/utils/type_conversion_utils.py
@@ -88,24 +88,15 @@ def get_schema_type_hint_from_dtype(dtype, array_values=None):
 
 def can_cast_to_float32(dtype, array_values):
     """
-    A dtype can be cast to float32 if it is a float type and converting it to float32 presents the same output as the
-    original values. Note that NaNs fail equality (i.e. np.NaN != np.NaN) so we use np.testing.assert_equal to ensure
-    that the arrays are equal minus NaNs.
+    Optimistically returns True signifying that a type downcast to float32 is possible whenever the incoming type is
+    a float.
 
     We also handle a special case here where the array is a Series object with integer categorical values AND NaNs.
-    Since NaNs are floating points in numpy, we upcast the integer array to float32.
+    Since NaNs are floating points in numpy, we upcast the integer array to float32 and return True.
     """
 
     if dtype.kind == "f":
-        # Try to convert the array to float32
-        converted_float32_values = array_values.to_numpy(np.float32)
-        original_values = array_values.to_numpy()
-
-        # Verify that the two arrays are equal except for NaNs (which will equate to be unequal).
-        if not ((converted_float32_values != original_values) == np.isnan(original_values)).all():
-            return False
-
-        if dtype != np.float32:
+        if not np.can_cast(dtype, np.float32):
             logging.warning(f"Type {dtype.name} will be converted to 32 bit float and may lose precision.")
 
         return True
@@ -138,9 +129,9 @@ def can_cast_to_int32(dtype, array_values=None):
             return True
         ii32 = np.iinfo(np.int32)
         if (
-            not ordered_array_values.empty
-            and (ordered_array_values.min() >= ii32.min and ordered_array_values.max() <= ii32.max)
-            or ordered_array_values.empty
+                not ordered_array_values.empty
+                and (ordered_array_values.min() >= ii32.min and ordered_array_values.max() <= ii32.max)
+                or ordered_array_values.empty
         ):
             return True
     return False