From ab1b9368a031822207a947a5a0336c5f59ace134 Mon Sep 17 00:00:00 2001 From: bmccandless Date: Tue, 25 Aug 2020 17:21:39 -0700 Subject: [PATCH 01/17] fix pca call in reembeddings (#1793) This had the wrong dim passed into n_comps, and so failed when the number of genes was less than 50. --- server/compute/scanpy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/compute/scanpy.py b/server/compute/scanpy.py index 36c2e1a9..309eb270 100644 --- a/server/compute/scanpy.py +++ b/server/compute/scanpy.py @@ -43,7 +43,7 @@ def scanpy_umap(adata, obs_mask=None, pca_options={}, neighbors_options={}, umap for k in list(adata.uns.keys()): del adata.uns[k] - sc.pp.pca(adata, zero_center=None, n_comps=min(adata.n_obs - 1, 50), **pca_options) + sc.pp.pca(adata, zero_center=None, n_comps=min(adata.n_vars - 1, 50), **pca_options) sc.pp.neighbors(adata, **neighbors_options) sc.tl.umap(adata, **umap_options) From f8cdb1289248e90000e2aeac823ebd84531c7c0f Mon Sep 17 00:00:00 2001 From: bmccandless Date: Wed, 26 Aug 2020 13:01:50 -0700 Subject: [PATCH 02/17] Fix frontend mishandling of null userinfo (#1795) * Fix frontend mishandling of null userinfo If the authentication is disabled, the userinfo endpoint returns null. This case needs to be handled. #1780 * Small fix for handling refesh tokens in auth --- client/src/actions/index.js | 2 +- server/auth/auth_oauth.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/client/src/actions/index.js b/client/src/actions/index.js index 4c39f86d..8730730e 100644 --- a/client/src/actions/index.js +++ b/client/src/actions/index.js @@ -43,7 +43,7 @@ async function configFetch(dispatch) { async function userInfoFetch(dispatch) { return fetchJson("userinfo").then((response) => { - const userinfo = { ...response.userinfo }; + const { userinfo } = response || {}; dispatch({ type: "userinfo load complete", userinfo, diff --git a/server/auth/auth_oauth.py b/server/auth/auth_oauth.py index 084d1af9..428d0048 100644 --- a/server/auth/auth_oauth.py +++ b/server/auth/auth_oauth.py @@ -29,7 +29,9 @@ class Tokens: self.id_token = id_token self.refresh_token = refresh_token self.expires_at = expires_at - if not (access_token and id_token and refresh_token and expires_at): + + # expires_at may be None after a token refresh, and so it is not checked here + if not (access_token and id_token and refresh_token): raise KeyError(str(self.__dict__)) From ed865e9a57a953303623f9cbdc1e1bca849748a8 Mon Sep 17 00:00:00 2001 From: maniarathi Date: Mon, 31 Aug 2020 16:16:21 -0700 Subject: [PATCH 03/17] Update the release process for community release to include release candidate versioning (#1802) --- .bumpversion.cfg | 14 ++ Makefile | 47 +++++-- dev_docs/release_process.md | 132 ++++++++++-------- server/cli/upgrade.py | 13 +- .../1e4dfec4-c0b2-46ad-a04e-ff3ffb3c0a8f.h5ad | Bin 0 -> 20352 bytes .../a95c59b4-7f5d-4b80-ad53-a694834ca18b.h5ad | Bin 0 -> 20352 bytes 6 files changed, 125 insertions(+), 81 deletions(-) create mode 100644 server/test/fixtures/1e4dfec4-c0b2-46ad-a04e-ff3ffb3c0a8f.h5ad create mode 100644 server/test/fixtures/a95c59b4-7f5d-4b80-ad53-a694834ca18b.h5ad diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 0d09f24d..3ec24118 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,19 @@ [bumpversion] current_version = 0.16.0 +commit = True +# The below regex details an acceptable version number by naming the groups (major, minor, patch, prerel, and +# prerelversion) and also specifying the valid values for each group (integers, `\d+`, for major, minor, patch, and +# prerelversion and only `rc` as the acceptable value for prerel). +parse = (?P\d+)\.(?P\d+)\.(?P\d+)(?:-(?Prc)\.(?P\d+))? +serialize = + {major}.{minor}.{patch}-{prerel}.{prerelversion} + {major}.{minor}.{patch} + +[bumpversion:part:prerel] +optional_value = release +values = + rc + release [bumpversion:file:setup.py] search = version="{current_version}" diff --git a/Makefile b/Makefile index 9c0eb686..5b588b77 100644 --- a/Makefile +++ b/Makefile @@ -99,22 +99,32 @@ pydist: build # RELEASE HELPERS -# create new version to commit to main -.PHONY: release-stage-1 -release-stage-1: dev-env bump clean-lite gen-package-lock +# Create new version to commit to main +.PHONY: create-release-candidate +create-release-candidate: dev-env bump-version clean-lite gen-package-lock @echo "Version bumped part:$(PART) and client built. Ready to commit and push" -# build dist and release to dev pypi -.PHONY: release-stage-2 -release-stage-2: dev-env pydist twine +# Bump the release candidate version if needed (i.e. the previous release candidate had errors). +.PHONY: recreate-release-candidate +recreate-release-candidate: dev-env bump-release-candidate clean-lite gen-package-lock + @echo "Version bumped part:$(PART) and client built. Ready to commit and push" + +# Build dist and release to Test PyPI +.PHONY: release-candidate-to-test-pypi +release-candidate-to-test-pypi: dev-env pydist twine @echo "Dist built and uploaded to test.pypi.org" @echo "Test the install:" @echo " make install-release-test" - @echo "Then upload to Pypi prod:" - @echo " make twine-prod" -.PHONY: release-stage-final -release-stage-final: twine-prod +# Build final dist (gets rid of the rc tag) and release final candidate to TestPyPI +.PHONY: release-final-to-test-pypi +release-final-to-test-pypi: dev-env bump-release clean-lite gen-package-lock pydist twine + @echo "Final release dist built and uploaded to test.pypi.org" + @echo "Test the install:" + @echo " make install-release-test" + +.PHONY: release-final +release-final: twine-prod @echo "Release uploaded to pypi.org" # DANGER: releases directly to prod @@ -136,11 +146,22 @@ dev-env-client: dev-env-server: pip install -r server/requirements-dev.txt -# give PART=[major, minor, part] as param to make bump -.PHONY: bump -bump: +# Set PART=[major, minor, patch] as param to make bump. +# This will create a release candidate. (i.e. 0.16.1 -> 0.16.2-rc.0 for a patch bump) +.PHONY: bump-version +bump-version: bumpversion --config-file .bumpversion.cfg $(PART) +# Increments the release candidate version (i.e. 0.16.2-rc.1 -> 0.16.2-rc.2) +.PHONY: bump-release-candidate +bump-release-candidate: + bumpversion --config-file .bumpversion.cfg prerelversion --allow-dirty + +# Finalizes the release candidate by removing the release candidate tag (i.e. 0.16.2-rc.2 -> 0.16.2). +.PHONY: bump-release +bump-release: + bumpversion --config-file .bumpversion.cfg prerel --allow-dirty + .PHONY: twine twine: twine upload --repository-url https://test.pypi.org/legacy/ dist/* diff --git a/dev_docs/release_process.md b/dev_docs/release_process.md index 6a02280c..f502a44c 100644 --- a/dev_docs/release_process.md +++ b/dev_docs/release_process.md @@ -1,4 +1,4 @@ -# cellxgene release process +# cellxgene Release Process _This document defines the release process for cellxgene_ @@ -16,71 +16,65 @@ The release process should result in the following side-effects: Note all release tags pushed to GitHub MUST follow semantic versioning. -## Recipe +## Releasing a Major or Minor Version of cellxgene -Follow these steps to create a release. +Please scroll down the section below for how to release a patch version. Follow these steps to create a major or minor release. 1. Preparation: - python3.6 environment, and a cellxgene clone - - Define the release version number, using [semantic versioning](https://semver.org/), - and specifying all three digits (eg, 0.3.0) - - Write the release title and release notes and add to - [release notes document](https://docs.google.com/document/d/1KnHwkYfhyWO5H8BDcMu7y3ogjvq5Yi4OwpmZ8DB6w0Y/edit) -2. Create a release branch, eg, `release-version` -3. In the release branch: - - Run `make release-stage-1 PART=[major | minor | patch]` where you choose major/minor/patch depending on which part of the version - is being bumped (eg, 0.2.9->0.3 is minor). -4. Commit and push the new branch -5. Create a PR for the release. - - [optional] As needed, conduct PR review. -6. Merge to the `main` branch -7. Publish to pypi by performing the following steps (assumes you that you have registered for pypi, - and that you have write access to the cellxgene pypi package): - - Build the distribution and upload to test pypi `make release-stage-2` - - Test the test installation in a fresh virtual environment using `make install-release-test` - - Upload the package to real pypi using `make release-stage-final` - - Test the installation in a fresh virtual environment using `pip install cellxgene` -8. Create Github release using the version number and release notes - ([instructions](https://help.github.com/articles/creating-releases/)). - - Draft new release - - Type version name matching release version number from (1) - - Select `main` as release branch (ensure you merged the release PR) - - Type title `Release {version num}` - - [optional] Check pre-release if this release is not ready for production - - Publish Release + - Define the release version number, using [semantic versioning](https://semver.org/), and specifying all three digits (e.g., 0.3.0) + - Write the release title and release notes and add to [release notes document](https://docs.google.com/document/d/1KnHwkYfhyWO5H8BDcMu7y3ogjvq5Yi4OwpmZ8DB6w0Y/edit) +2. Create a release branch, eg, `release-version-0.16.0` +3. In the release branch, run `make create-release-candidate PART=[major | minor | patch]` where you choose major/minor/patch depending on which part of the version is being bumped (e.g., `0.2.9` -> `0.3.0` is minor version bump). This will bump the version and create a release *candidate* version (i.e. `0.3.0-rc.0`). +4. Commit and push the new branch. This will trigger tests to ensure that your branch isn't broken. +5. Upload the release candidate to Test PyPI by running the command `make release-candidate-to-test-pypi`. (Make sure you are registered for PyPI and Test PyPI and you have write access to the cellxgene PyPI package for both). +6. Verify the release candidate in a fresh virtual environment by running `make install-release-test` which installs the cellxgene build you just uploaded the Test PyPI. +7. If you find errors with the release candidate, run `make recreate-release-candidate` to increment the release candidate version (i.e. `0.3.0-rc.0` -> `0.3.0-rc.1`). Then go back to Steps 5 and 6 to re-upload and re-test the new release candidate. +8. If everything looks good, push the release to Test PyPI without the release candidate tag by running the command `make release-final-to-test-pypi` (i.e. `0.3.0-rc.1` -> `0.3.0`). + - **NOTE:** Once you push the final release version to Test PyPI, you cannot ever re-upload the build again. If you need to make changes to the build, you will have to "burn" the version number and bump the part again and go back to step 1 with a brand new version number. For example, if you upload `0.3.0` to Test PyPI and realize there's a bug, you will have to create a new version `0.4.0` and there will be no `0.3.0` version of cellxgene. This is why testing the release candidate is very important. +9. Create a PR for the release and conduct a PR review. +10. Merge to the `main` branch. +11. Publish to PyPI (prod) (assuming you that you have registered for PyPI, and that you have write access to the cellxgene pypi package) by running `make release-final`. +12. Test the installation in a fresh virtual environment by running `pip install --no-cache-dir cellxgene`. +13. Create Github release using the version number and release notes ([instructions](https://help.github.com/articles/creating-releases/)): + - Draft new release + - Type version name matching release version number from (1) + - Select `main` as release branch (ensure you merged the release PR) + - Type title `Release {version num}` + - [optional] Check pre-release if this release is not ready for production + - Publish Release -The optional steps are for testing purposes, and are recommended -for publishing any major releases, and any releases that significantly -change the packaging (e.g. new bundled files, new dependencies, etc.) +The optional steps are for testing purposes, and are recommended for publishing any major releases, and any releases that significantly change the packaging (e.g. new bundled files, new dependencies, etc.) -### Point release (special case) +### Releasing a Patch Version of cellxgene (special case) -To make a bugfix release (a point release) when there are already other changes in `main` we need to do a modified version of our release process. The difference is that instead of using `main` we are going make our release branch off of the tag for the release we want to patch. We cherrypick the commits that we want to include in the patch. Then instead of merging to `main`, we create the release directly off of the branch. +To make a bugfix release (a point release/patch release) when there are already other changes in `main` we need to do a modified version of our release process. The difference is that instead of using `main` we are going make our release branch off of the tag for the release we want to patch. We cherrypick the commits that we want to include in the patch. Then instead of merging to `main`, we create the release directly off of the branch. 1. (same as above) Preparation: - python3.6 environment, and a cellxgene clone - - Define the release version number, using [semantic versioning](https://semver.org/), - and specifying all three digits (eg, 0.3.0) (for this you will update the last digit to represent a bugfix change) - - Write the release title and release notes and add to - [release notes document](https://docs.google.com/document/d/1KnHwkYfhyWO5H8BDcMu7y3ogjvq5Yi4OwpmZ8DB6w0Y/edit) + - Define the release version number, using [semantic versioning](https://semver.org/), and specifying all three digits (e.g., 0.3.2) (for this you will update the last digit to represent a bugfix change). + - Write the release title and release notes and add to [release notes document](https://docs.google.com/document/d/1KnHwkYfhyWO5H8BDcMu7y3ogjvq5Yi4OwpmZ8DB6w0Y/edit) 2. Create a release branch off of the tag for the release you want to update. - - Checkout the tag for the release you want to fix. ex. if we are fixing 0.9.0: `git checkout 0.9.0` - - Create a branch from that tag. `git branch release-0.9.1` + - Checkout the tag for the release you want to fix. For example, if we are fixing 0.9.0: `git checkout 0.9.0`. + - Create a branch from that tag. `git branch release-version-0.9.1` 3. Cherrypick the commits that you want included in this patch. - - Test that the cherrypicked commits landed and fixed the issue - - We WILL NOT merge this branch back into `main`, these commits should already exist in `main`. -4. In the release branch: - - Run `make release-stage-1 PART=patch`. -5. Commit and push the new branch. DO NOT MAKE A PR OR MERGE TO `main`. - - wait for release to pass the tests -6. Publish to pypi by performing the following steps (assumes you that you have registered for pypi, - and that you have write access to the cellxgene pypi package): - Build the distribution and upload to test pypi `make release-stage-2` - Test the test installation in a fresh virtual environment using `make install-release-test` - Upload the package to real pypi using `make release-stage-final` - Test the installation in a fresh virtual environment using - `pip install --no-cache-dir cellxgene` -7. Create Github release using the version number and release notes + - Test that the cherrypicked commits landed and fixed the issue locally. + - We **WILL NOT** merge this branch back into `main` as these commits should already exist in `main`. +4. In the release branch (i.e. `release-version-0.9.1`), run `make create-release-candidate PART=patch` to bump the patch version and create the first release candidate (i.e. `0.9.1-rc.0`). +5. Run `make release-candidate-to-test-pypi` to upload the release candidate to Test PyPI. +6. Verify the release candidate in a fresh virtual environment by running `make install-release-test` which installs the cellxgene build you just uploaded the Test PyPI. +7. If you find errors with the release candidate, run `make recreate-release-candidate` to increment the release candidate version (i.e. `0.9.1-rc.0` -> `0.9.1-rc.1`). Then go back to Steps 5 and 6 to re-upload and re-test the new release candidate. +8. If everything looks good, push the final version of the release to Test PyPI without the release candidate tag by running the command `make release-final-to-test-pypi` (i.e. `0.9.1-rc.1` -> `0.9.1`). + - **NOTE:** Once you push the final release version to Test PyPI, you cannot ever re-upload the build again. If you need to make changes to the build, you will have to "burn" the version number and bump the part again and go back to step 1 with a brand new version number. For example, if you upload `0.9.1` to Test PyPI and realize there's a bug, you will have to create a new version `0.9.2` and there will be no `0.9.1` version of cellxgene. This is why testing the release candidate is very important. +9. Commit and push the new branch. DO NOT MAKE A PR OR MERGE TO `main`. + - Wait for release to pass the tests. +10. Publish to PyPI (prod) (assuming you that you have registered for PyPI, and that you have write access to the cellxgene pypi package) by running `make release-final`. +11. Test the installation in a fresh virtual environment by running `pip install --no-cache-dir cellxgene`. +12. Create Github release using the version number and release notes ([instructions](https://help.github.com/articles/creating-releases/)). - Draft new release - Type version name matching release version number from (1) - - _Different than above_ Select the release-branch you pushed at step 5 as release branch + - [**_Different than above_**] Select the release-branch you pushed at step 5 as release branch - Type title `Release {version num}` - [optional] Check pre-release if this release is not ready for production - Publish Release @@ -93,36 +87,52 @@ _PyPi doesn't allow you to reupload a release with the same version number_ If you accidentally burned a release number you want to use on prod, you have a few options: 1. OPTION 1: Create distribution `make pydist`; test release locally `pip install dist/`; - then upload to prod `make release-stage-final`. + then upload to prod `make release-final`. 2. OPTION 2: (DANGER) release directly to prod: `make release-directly-to-prod`. -3. OPTION 3: If the release was burned on prod as well run from Step 3 again with option - PART=patch until you get to an unburned version. +3. OPTION 3: If the release was burned on prod as well run from Step 3 again with option PART=patch until you get to an unburned version. ### The release doesn't install or fails your tests when you install it Delete it from pypi - Go to pypi.org -> sign in -> go to the cellxgene package -> click manage -> then in the options drop down click delete -> follow the instructions. You will not be able to use that release number again. If it is a minor bug and not a major regression, you can just release a patch. -### If you need to run stage final on a different computer than stage 2 +### If you need to run the final upload to PyPI (prod) on a different computer than where you ran the command to upload to Test PyPI. -If you run stage final without running stage 2 first, the dist will not have been build on the computer running stage final. The solution is to run `make release-directly-to-prod`. This both builds the distribution files and then releases directly to prod pypi.org. +If you run `make release-final` without running `make release-final-to-test-pypi` first, the dist will not have been build on the computer running the final PyPI push. The solution is to run `make release-directly-to-prod`. This both builds the distribution files and then releases directly to prod pypi.org. -## Stage Details +## Command Details -### Stage 1 - `make release-stage-1` +### Initial creation stage - `make create-release-candidate PART=[major | minor | patch]` 1. Pip installs requirements-dev - 2. Bumps version by [PART] + 2. Bumps version by [PART] and creates the first release candidate. 3. Deletes build directory, client/build, dist and cellxgene.egg-info 4. Creates the package-lock.json -### Stage 2 - `make release-stage-2` +### Test PyPI upload stage - `make release-candidate-to-test-pypi` 1. Pip installs requirements-dev 2. Builds client and server 3. Creates distribution release (sdist) 4. Uploads to test.pypi.org + +### Recreating release candidate stage(s) - `make recreate-release-candidate` -### Stage final - `make release-stage-final` + 1. Pip installs requirements-dev + 2. Bumps release candidate version number. + 3. Deletes build directory, client/build, dist and cellxgene.egg-info + 4. Creates the package-lock.json + +### Penultimate stage, final release to Test PyPI - `make release-final-to-test-pypi` + 1. Pip installs requirements-dev + 2. Removes release candidate tag from the version number. + 3. Deletes build directory, client/build, dist and cellxgene.egg-info + 4. Creates the package-lock.json + 5. Pip installs requirements-dev + 6. Builds client and server + 7. Creates distribution release (sdist) + 8. Uploads to test.pypi.org + +### Final stage - `make release-final` ** Does not build distribution ** 1. Uploads to pypi.org diff --git a/server/cli/upgrade.py b/server/cli/upgrade.py index 53cf15cd..9ceda567 100644 --- a/server/cli/upgrade.py +++ b/server/cli/upgrade.py @@ -1,17 +1,16 @@ -import click import re -import requests +import click +import requests from requests.exceptions import ConnectionError + from .. import __version__ # Official SemVer regex: https://semver.org/ SEMVER_FORMAT = re.compile( - r"^(?P0|[1-9]\d*)\.(?P0|[1-9]\d*)\.(?P0|[1-9]\d*)" - + r"(?:-(?P(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)" - + r"(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?" - + r"(?:\+(?P[0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$" -) + r"^(?P0|[1-9]\d*)\.(?P0|[1-9]\d*)\.(?P0|[1-9]\d*)(?:-(?P(?:0|[1-9]\d*|\d*[" + r"a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+(?P[0-9a-zA-Z-]+(" + r"?:\.[0-9a-zA-Z-]+)*))?$") def log_upgrade_check(): diff --git a/server/test/fixtures/1e4dfec4-c0b2-46ad-a04e-ff3ffb3c0a8f.h5ad b/server/test/fixtures/1e4dfec4-c0b2-46ad-a04e-ff3ffb3c0a8f.h5ad new file mode 100644 index 0000000000000000000000000000000000000000..eee290364b3ea2bdbf61b4be6ed056659772c0e5 GIT binary patch literal 20352 zcmeHPU2GIp6u#SntWcnZDg{v|eK7ThS`-5k&2EKamGYAo0>sykeC=A)i>UViHSbdd+s@BXXb8aSJrOJ!o7sK zbI!T<%(>tFJNL{^KkMIrVEx){YXzieRIC!M>Xt7HlrA1Y4$~|jrke=mGn8L#!kdVI ze?oK+zm4?k<&F*Z^$A&$^>a?uM9(U`V?E3_`Qw2=e_!uWXz596ZV%=2G|q@D$(@l2 z#X6gGRW9iq+)U}&P&>?HR~CV={k8ZM*~Js4qM7$&C|>$p4PzSQ zux9w_H5&bdMuSu0FaM;ZMI>_j)e#m@uKXUmeCmtC1MNS>zTf^)vsjpq-M^8$ z@Xg@QF}0FW6WgT@KMlPA&0_!OX^n=}$u9>retN4$Z`0^!B>kNH&-%B^+Y>(eS-3aJ zAYc$M2p9wm0y+ZvxhhTtEOy+9PGs_FyC_5=kxCZqQOC{L>L4{ya5MQ)W=@OKStnWW zNZfFx4%#0{n-Cla(9Z!aq8~S6u|c%UQ-+;SIcfQX9fj#}Th`txcW5xJw3++PL!eyw z2=p-6*jvO=($^N$ch+_%GEN@qK&qzKzo~<>&?3)W%3%=erQQyaam> zs!!SviI1(EZv-aU4SF~DXmCM|p+3nwHh3H|B|w>?=+fmUs@_*S*B410HKwXQQ{d4; zfaeQs339)K9Z#+|6u08|P}uojkDG;|^X|SQ18OXzdY+!RkIG8vWFdLlP3CM;!1ELL z5+QoJcXq>C#`}TVwC)Vx7VZ$5-r0edv_w6yaArO z(mvuVzd+uzo)O~Xiw}>VB>pjU6$ecnY`ay5`yvJ zb~6stJtg}Y2MA(7;`q~Ps``HT;R3rpo09|&AP;`Yd1RsG6gB<6EFA&>!y?R1m* zv?C+z6mBi!^ElSaLx2O4&WAt;B%Kcd4@f#60w0i6Fhu6(I?*E zq4yK(WDF)Mf^Ug1E&z&n+<5)nj|!LQTI>6ZHtTt1zYvz#5*1r6kUinJTqj(D2YVeC z8L?ZUmer=FTAbOUz!1lmYI};Q)u;&Q>p#Z>Es8U9yEqQ1$1C;tCeQCKt{9T@`(OR{ znBIGt|Ns6MVz~8q{|hsIso(#qRfINQh@q?aG0||p6z_jAw#+-cRJus2wtjLPpYz(^ z)7_x;Gaj(wVOu}ZPiZc_S7_x460q7MEV?anAl?%N?s&Z&<3N5u`sfi4KCV1J1;?7D zKndTE7ic^&vWQ{%c)Ey!6}~<=9$m_J`mgV&!*SY3ufli+hv77~tO%9kfjfx)odv&t zWN*}?YyHk*GvN`O*yZD9dGV;`xApA7?x8`ytbJu3w(@@t8P8DE3jfPRMQUyvsx6kaVP}qn<5h!2pa3*uV)b3a9qxG!XJ~$I<+e!$+ ze#fGJ*M;k1Q|Nc1(KuR9nb>m}X|qlu4v+ek*TrF)NLQ)BcL(a*^04tc{x&}M5`AG+ zZC^jyi{;IDt#5xltlD|}o+Ja~lwYIq!*-_}cieH4iHX#hol7Q4zh-AX|8CxsU`^g% z^e6!T&Y5?Gx%c@=i|1T_ZvWzP=CW5k-^ZNA^Ek2Z9#~x7C|ty`Nmyj?>^qe$=XqXy zvvRq|Os~;ZM4IY8{{iDGnfX6Qli3hKDd@w3x3Rzn+v&m`OO{DFKl$#kZ$Pa+kSIk0w z|2LY)(ao#-DwpH)RX@dH O7;il;Q&~&>r2hi#%6FFl literal 0 HcmV?d00001 diff --git a/server/test/fixtures/a95c59b4-7f5d-4b80-ad53-a694834ca18b.h5ad b/server/test/fixtures/a95c59b4-7f5d-4b80-ad53-a694834ca18b.h5ad new file mode 100644 index 0000000000000000000000000000000000000000..9d7ffcc6af1e3acad02acc88949d4ee93c031842 GIT binary patch literal 20352 zcmeHPO>9(E6u#2}9ic!ADCI|e=|bxdO;HR#QD-VGRw>X>h!|t0)0sy*GBdAvowm~; zk;F|+OeAqrcSIL9x{!oTbt4H2NrVLB0undIs4EvncW&^Wd(N5n-krV~89Ob7dkOEp zbI!fzo%`LtbI+UC5BhoztzWx+t$-AbidCXT-SXuQrSr#-!!*l>=q5t>4CQkTcoPxu zPlz_+w~~In+==78y+W2`{hU)Z(X|TiSP%0}{$L=`*V{b+Ej>le?V|h|jWZ%ka%Xs4 zu_lwQ$|aqHn~6N~dAxQgY4{}~)~NA|7Z{;fBhr8x8mTG`Pf<>{sj973Ohi@LxY)B! z?72e)S4%EvYG^=(QJN`@5h^Z*=n0r@A~w`%x^*}q#ArZXZ({K1v7U$kt1(&J;E)IN zYEoQ{1sL~>MDucEQT;1hgXLylKze>J=7_!!GY5j@E)jk6Mdb53F{|nO`5BapPRf@l zi-;!G?`&ra0Z}BtW(lZj>!P(88(=L{JIrI37lE+-wfHdE#S^BYnSaJmy!g2q#x%%b z&G6Gr8ofrN!71^Ve@N0I61j1`KYru-hp`*io8x{l%mhEZNuyzA`Q@#WhSdvru#3PF z9+7yqZ2R)3$)2xbe}D9K@2zisi$w>rJNAG6UF_wnJO8qqeu@41?iZi@?)(r-cYgZM zA5VTCQ!5!Yaf{U9r=b_1S?vEjqS3HA`Q@O-PjA!c?Hav9(vQjitbeDxeZxmT3il=% z1PlTO0fT@+Ku177SH-D-#g04CiA+9i7llY9Qptin;m&;v zi5sreLHi?V6N2La`Z=Ik^x;MsiP&GOu(90svo>TMG#Cp(eL?{eIH>9v6@Ww^SKc}k6;H@D zL`_^fE!w03@Z1)u1^YNE4t4hr_DbG~<0m0g0+cC=E**ZN>V36yeUa2rW2)*i1@12d zc)rki9JU?+S z5u&SecPFf6ydS82OVYX@!0}PF&q=xNH?TgnUr9OdZ%p%idrrzA(K1}JcQ>-h8{oMs z?IXVO3*!6!{i15gMdN6AYc$M2p9wm0tNwtfI(o15zyoRd``2paF%{v=W}Je zMtd=?TKbWb8071wavx8?V3nP~jq7YkhywYCW#(7s3)-qhjlOWKTFQmkF2P!CuEj zM(h@;WwoiP7H767FvRht+MZ%+tyBc`^`GN`7R8ylT^xthAt1)Gaj(wURyuWPiZW@S7_l060q7MEV?anAl?%N?s&Z&;quxBKhc^04tc_7Xn#0)1gs zZC@Yg#`0#o*0;azRqZ@}PmzIf$}iFQVY^d~JLb5_#CU4d&LtD2U$Zlxe>d+*uqN*> zdK7?v=ghmp{OkOrg>$Yy-?Olsx$IS6>t)Wud7Rj{4=pTTDO|*{L0Dw)>?@Tm=XqY- zU%A|aCas@;G`K?jBmM6X;Kg z`@fN7K7)TF3hFYQaPkEw>x@h%3bva|WYVN~%yr(eQw8m|Z%2T^{@c7Ny>c#i&RH$S$mF@#D%(Cw;%kR1mm4DSY+vPa9NnBdSh*aZulgts O!+7g)naWz~C;b-=n`vYK literal 0 HcmV?d00001 From 54b42607aef6a594461a7842d9edf4c657e4398a Mon Sep 17 00:00:00 2001 From: bmccandless Date: Mon, 31 Aug 2020 18:26:35 -0700 Subject: [PATCH 04/17] Update the location of deployment assets for the eb server (#1806) put deploy scripts in /static/cellxgene/deploy instead of /static/deploy fixed chanzuckerberg/corpora-data-portal#558 --- server/eb/Makefile | 3 ++- server/eb/README.md | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/server/eb/Makefile b/server/eb/Makefile index a51ccdce..3c9c5b4a 100644 --- a/server/eb/Makefile +++ b/server/eb/Makefile @@ -34,7 +34,8 @@ build: clean cp customize/requirements.txt artifact.dir; \ fi ; \ if [ -d customize/deploy ] ; then \ - cp -r customize/deploy artifact.dir/server/common/web/static; \ + mkdir -p artifact.dir/server/common/web/static/cellxgene; \ + cp -r customize/deploy artifact.dir/server/common/web/static/cellxgene; \ fi; \ if [ -d customize/inline_scripts ] ; then \ cp -r customize/inline_scripts/* artifact.dir/server/common/web/templates; \ diff --git a/server/eb/README.md b/server/eb/README.md index 2e7827c9..f4209f15 100644 --- a/server/eb/README.md +++ b/server/eb/README.md @@ -95,7 +95,7 @@ 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/deploy/ +* modify your configuration file to set the location to these file: /static/cellxgene/deploy/ 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. @@ -106,9 +106,9 @@ $ cp /tos.html customize/deploy/tos.html $ cp /privacy.html customize/deploy/privacy.html # edit config.yaml -$ grep "/static/deploy" config.yaml -about_legal_tos: /static/deploy/tos.html -about_legal_privacy: /static/deploy/privacy.html +$ grep "/static/cellxgene/deploy" config.yaml +about_legal_tos: /static/cellxgene/deploy/tos.html +about_legal_privacy: /static/cellxgene/deploy/privacy.html ``` #### Inline javascript scripts From 437fd5fedae4e312d510ef80592bb9f05c6cdb62 Mon Sep 17 00:00:00 2001 From: Severiano Badajoz Date: Tue, 1 Sep 2020 16:45:50 -0700 Subject: [PATCH 05/17] Correctly check if mini histograms shouldn't be rendered (#1809) * ensure that function returns a boolean value * change function used to check if mini histogram should not render --- client/src/components/categorical/value/index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/client/src/components/categorical/value/index.js b/client/src/components/categorical/value/index.js index c04af970..f71df03e 100644 --- a/client/src/components/categorical/value/index.js +++ b/client/src/components/categorical/value/index.js @@ -81,7 +81,7 @@ class CategoryValue extends React.Component { get shouldRenderStackedBarOrHistogram() { const { colorAccessor, isColorBy, annotations } = this.props; - return colorAccessor && !isColorBy && !annotations.isEditingLabelName; + return !!colorAccessor && !isColorBy && !annotations.isEditingLabelName; } handleDeleteValue = () => { @@ -439,7 +439,9 @@ class CategoryValue extends React.Component { if ( !this.shouldRenderStackedBarOrHistogram || - !AnnotationsHelpers.isContinuousAnnotation(schema, colorAccessor) + // This function returns true on categorical annotations(when stacked bar should not render), + // in cases where the colorAccessor is a gene this function will return undefined since genes do not live on the schema + AnnotationsHelpers.isCategoricalAnnotation(schema, colorAccessor) === true ) { return null; } From 0a27b2923a8ea3d959bbbbd1ab5f6abce652f8e2 Mon Sep 17 00:00:00 2001 From: bmccandless Date: Wed, 2 Sep 2020 15:35:50 -0700 Subject: [PATCH 06/17] =?UTF-8?q?Add=20error=20message=20and=20exit=20if?= =?UTF-8?q?=20reembeddings=20is=20enabled=20and=20scanpy=20is=20n=E2=80=A6?= =?UTF-8?q?=20(#1812)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add error message and exit if reembeddings is enabled and scanpy is not installed fixes #1811 --- server/common/app_config.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/server/common/app_config.py b/server/common/app_config.py index df7a0f96..ae67eeaf 100644 --- a/server/common/app_config.py +++ b/server/common/app_config.py @@ -9,6 +9,7 @@ import yaml from flatten_dict import flatten, unflatten import server.compute.diffexp_cxg as diffexp_tiledb +import server.compute.scanpy from server import display_version as cellxgene_display_version from server.auth.auth import AuthTypeFactory from server.common.annotations.hosted_tiledb import AnnotationsHostedTileDB @@ -898,16 +899,21 @@ class DatasetConfig(BaseConfig): self.check_attr("embeddings__enable_reembedding", bool) server_config = self.app_config.server_config - if server_config.single_dataset__datapath: - if self.embeddings__enable_reembedding: + if self.embeddings__enable_reembedding: + if server_config.single_dataset__datapath: matrix_data_loader = MatrixDataLoader( server_config.single_dataset__datapath, app_config=self.app_config ) if matrix_data_loader.matrix_data_type != MatrixDataType.H5AD: - raise ConfigurationError("'enable-reembedding is only supported with H5AD files.") + raise ConfigurationError("enable-reembedding is only supported with H5AD files.") if server_config.adaptor__anndata_adaptor__backed: raise ConfigurationError("enable-reembedding is not supported when run in --backed mode.") + try: + server.compute.scanpy.get_scanpy_module() + except NotImplementedError: + raise ConfigurationError("Please install scanpy to enable UMAP re-embedding") + def handle_diffexp(self, context): self.check_attr("diffexp__enable", bool) self.check_attr("diffexp__lfc_cutoff", float) From 5781879da52955e3891d46d59bffa42351fbfa53 Mon Sep 17 00:00:00 2001 From: Ambrose J Carr Date: Wed, 2 Sep 2020 20:22:31 -0400 Subject: [PATCH 07/17] remove core team section (#1798) --- README.md | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/README.md b/README.md index 18f6fdc3..e2cb3215 100644 --- a/README.md +++ b/README.md @@ -79,23 +79,7 @@ This project was started with the sole goal of empowering the scientific communi If you believe you have found a security issue, we would appreciate notification. Please send email to . -# About - -### Core team - -The current core team: - -- Colin Megill, frontend & product design -- Bruce Martin, software engineer -- Sidney Bell, computational biologist -- Lia Prins, designer -- Severiano Badajoz, software engineer - -We would also like to gratefully acknowledge contributions from past core team members: - -- Charlotte Weaver, software engineer - -### Inspiration +# Inspiration We've been heavily inspired by several other related single-cell visualization projects, including the [UCSC Cell Browswer](http://cells.ucsc.edu/), [Cytoscape](http://www.cytoscape.org/), [Xena](https://xena.ucsc.edu/), [ASAP](https://asap.epfl.ch/), [Gene Pattern](http://genepattern-notebook.org/), and many others. We hope to explore collaborations where useful as this community works together on improving interactive visualization for single-cell data. From 89b68723cc9f398ddc095ef15ff7515a4dd61b26 Mon Sep 17 00:00:00 2001 From: Severiano Badajoz Date: Wed, 9 Sep 2020 17:55:43 -0700 Subject: [PATCH 08/17] Create dataset info drawer (#1805) * create infoDrawer * create read/writes to redux store * reimplement reducer that vanished * remove aboutURL stuff from title * add formatting and style * s/length/size and make metadata items list items * remove comment * remove empty singletons * refactor into async react component * Clean up skeleton * swap out for loop for map * add comment * replace placeholder * switch ternary for `&&` * event handling fixes and PR feedback * add button and move click handler to button * ditch empty categories * move drawer button handling to redux * remove categorical move note * PR feedback from colin * update snapshot * remove hover state --- .../e2e/__snapshots__/e2e.test.js.snap | 2 +- .../components/categorical/category/index.js | 14 +- .../src/components/infoDrawer/infoDrawer.js | 152 ++++++++++++++++++ .../leftSidebar/topLeftLogoAndTitle.js | 57 +++---- client/src/components/menubar/index.js | 5 +- client/src/components/menubar/infoMenu.js | 22 +-- client/src/reducers/controls.js | 9 ++ 7 files changed, 198 insertions(+), 63 deletions(-) create mode 100644 client/src/components/infoDrawer/infoDrawer.js diff --git a/client/__tests__/e2e/__snapshots__/e2e.test.js.snap b/client/__tests__/e2e/__snapshots__/e2e.test.js.snap index fce4a085..e7207fb7 100644 --- a/client/__tests__/e2e/__snapshots__/e2e.test.js.snap +++ b/client/__tests__/e2e/__snapshots__/e2e.test.js.snap @@ -1,5 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`did launch page launched 1`] = `"pbmc3kc3k"`; +exports[`did launch page launched 1`] = `"pbmc3kc3k"`; exports[`metadata loads categories and values from dataset appear 1`] = `"
louvainvain
"`; diff --git a/client/src/components/categorical/category/index.js b/client/src/components/categorical/category/index.js index 90999deb..6fc628f1 100644 --- a/client/src/components/categorical/category/index.js +++ b/client/src/components/categorical/category/index.js @@ -491,19 +491,7 @@ const CategoryRender = React.memo( /* Entire category has a single value, special case. */ - const theOneValue = categorySummary.categoryValues[0]; - return ( -
- - - {metadataField} - - - - {`: ${theOneValue}`} - -
- ); + return null; } /* diff --git a/client/src/components/infoDrawer/infoDrawer.js b/client/src/components/infoDrawer/infoDrawer.js new file mode 100644 index 00000000..047e5017 --- /dev/null +++ b/client/src/components/infoDrawer/infoDrawer.js @@ -0,0 +1,152 @@ +import React, { PureComponent } from "react"; +import { connect, shallowEqual } from "react-redux"; +import { Drawer, H3, H1, UL, Classes } from "@blueprintjs/core"; +import Async from "react-async"; +import { + selectableCategoryNames, + createCategorySummaryFromDfCol, +} from "../../util/stateManager/controlsHelpers"; + +@connect((state) => { + return { + annoMatrix: state.annoMatrix, + schema: state.annoMatrix.schema, + datasetTitle: state.config?.displayNames?.dataset ?? "", + aboutURL: state.config?.links?.["about-dataset"], + isOpen: state.controls.datasetDrawer, + }; +}) +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) => { + 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; + + dispatch({ type: "toggle dataset drawer" }); + }; + + render() { + const { position, aboutURL, datasetTitle, schema, isOpen } = this.props; + + return ( + + + + + + + {(error) => { + console.error(error); + return Failed to load info; + }} + + + {(asyncProps) => { + const { singleValueCategories } = asyncProps; + return ( + + ); + }} + + + + ); + } +} + +const NUM_CATEGORIES = 8; + +const singleValueCategoriesPlaceholder = Array.from(Array(NUM_CATEGORIES)).map( + (_, index) => { + return [index, index]; + } +); + +const InfoFormat = ({ + datasetTitle, + singleValueCategories = new Map(singleValueCategoriesPlaceholder), + aboutURL = "thisisabouthtelengthofaurl", + skeleton = false, +}) => { + return ( +
+

{datasetTitle}

+ {singleValueCategories.size > 0 && ( + <> +

+ Dataset Metadata +

+
    + {Array.from(singleValueCategories).map((pair) => { + if (!pair[1] || pair[1] === "") return null; + return ( +
  • {`${pair[0]}: ${pair[1]}`}
    • + ); + })} +
+ + )} + {aboutURL && ( + <> +

More Info

+ + {aboutURL} + + + )} +
+ ); +}; +export default InfoDrawer; diff --git a/client/src/components/leftSidebar/topLeftLogoAndTitle.js b/client/src/components/leftSidebar/topLeftLogoAndTitle.js index ee274b6f..f2346593 100644 --- a/client/src/components/leftSidebar/topLeftLogoAndTitle.js +++ b/client/src/components/leftSidebar/topLeftLogoAndTitle.js @@ -1,22 +1,27 @@ // jshint esversion: 6 import React from "react"; import { connect } from "react-redux"; +import { Button } from "@blueprintjs/core"; +import { IconNames } from "@blueprintjs/icons"; + import * as globals from "../../globals"; import Logo from "../framework/logo"; import Truncate from "../util/truncate"; +import InfoDrawer from "../infoDrawer/infoDrawer"; -const DATASET_TITLE_WIDTH = 190; const DATASET_TITLE_FONT_SIZE = 14; @connect((state) => ({ datasetTitle: state.config?.displayNames?.dataset ?? "", - aboutURL: state.config?.links?.["about-dataset"], - scatterplotXXaccessor: state.controls.scatterplotXXaccessor, - scatterplotYYaccessor: state.controls.scatterplotYYaccessor, })) class LeftSideBar extends React.Component { + handleClick = () => { + const { dispatch } = this.props; + dispatch({ type: "toggle dataset drawer" }); + }; + render() { - const { datasetTitle, aboutURL } = this.props; + const { datasetTitle } = this.props; return (
gene -
- {aboutURL ? ( - - - {datasetTitle} - - - ) : ( - - - {datasetTitle} - - - )} -
+ + + {datasetTitle} + + + +
); } diff --git a/client/src/components/menubar/index.js b/client/src/components/menubar/index.js index c06ece70..3fd840c0 100644 --- a/client/src/components/menubar/index.js +++ b/client/src/components/menubar/index.js @@ -250,10 +250,7 @@ class MenuBar extends React.PureComponent { > { + dispatch({ type: "toggle dataset drawer" }); +}; + const InformationMenu = React.memo((props) => { - const { libraryVersions, aboutLink, tosURL, privacyURL } = props; + const { libraryVersions, tosURL, privacyURL, dispatch } = props; return (
- {aboutLink ? ( - - ) : ( - "" - )} + handleClick(dispatch)} + icon={IconNames.BOOK} + text="Dataset Overview" + /> { @@ -162,6 +165,12 @@ const Controls = ( scatterplotYYaccessor: null, }; + /************************** + Dataset Drawer + **************************/ + case "toggle dataset drawer": + return { ...state, datasetDrawer: !state.datasetDrawer }; + default: return state; } From 5583e913926c62485bad5f24cea88a3fcc24f2e8 Mon Sep 17 00:00:00 2001 From: Severiano Badajoz Date: Thu, 10 Sep 2020 16:41:05 -0700 Subject: [PATCH 09/17] Pull config values into dataset overview drawer (#1814) This PR adds multiple data to the dataset overview drawer provided by the config endpoint and formats them accordingly. The appearance of this new data is contingent on `dataPortalProps.corpora_schema_version === "1.0.0"` For QA launch cellxgene with a remixed dataset and click on the button in the upper left-hand corner or the updated button in the info menu. ![image](https://user-images.githubusercontent.com/8716829/92670435-de966280-f2c8-11ea-87f1-8591c959a586.png) ~~Review opening is blocked by merge of #1805~~ --- Closes #1319 --- client/configuration/eslint/eslint.js | 1 + .../src/components/infoDrawer/infoDrawer.js | 78 ++----- .../src/components/infoDrawer/infoFormat.js | 194 ++++++++++++++++++ client/src/components/termsPrompt/index.js | 4 +- client/src/reducers/controls.js | 1 - 5 files changed, 219 insertions(+), 59 deletions(-) create mode 100644 client/src/components/infoDrawer/infoFormat.js diff --git a/client/configuration/eslint/eslint.js b/client/configuration/eslint/eslint.js index 21fbfe8d..bafb7d31 100644 --- a/client/configuration/eslint/eslint.js +++ b/client/configuration/eslint/eslint.js @@ -39,6 +39,7 @@ module.exports = { }, }, rules: { + "react/jsx-no-target-blank": "off", "eslint-comments/require-description": ["error"], "no-magic-numbers": "off", "no-nested-ternary": "off", diff --git a/client/src/components/infoDrawer/infoDrawer.js b/client/src/components/infoDrawer/infoDrawer.js index 047e5017..49189ff2 100644 --- a/client/src/components/infoDrawer/infoDrawer.js +++ b/client/src/components/infoDrawer/infoDrawer.js @@ -1,7 +1,9 @@ import React, { PureComponent } from "react"; import { connect, shallowEqual } from "react-redux"; -import { Drawer, H3, H1, UL, Classes } from "@blueprintjs/core"; +import { Drawer } from "@blueprintjs/core"; import Async from "react-async"; + +import InfoFormat from "./infoFormat"; import { selectableCategoryNames, createCategorySummaryFromDfCol, @@ -14,6 +16,7 @@ import { datasetTitle: state.config?.displayNames?.dataset ?? "", aboutURL: state.config?.links?.["about-dataset"], isOpen: state.controls.datasetDrawer, + dataPortalProps: state.config?.["corpora_props"] ?? {}, }; }) class InfoDrawer extends PureComponent { @@ -60,7 +63,14 @@ class InfoDrawer extends PureComponent { }; render() { - const { position, aboutURL, datasetTitle, schema, isOpen } = this.props; + const { + position, + aboutURL, + datasetTitle, + schema, + isOpen, + dataPortalProps, + } = this.props; return ( - + {(error) => { @@ -87,7 +100,12 @@ class InfoDrawer extends PureComponent { const { singleValueCategories } = asyncProps; return ( ); }} @@ -97,56 +115,4 @@ class InfoDrawer extends PureComponent { ); } } - -const NUM_CATEGORIES = 8; - -const singleValueCategoriesPlaceholder = Array.from(Array(NUM_CATEGORIES)).map( - (_, index) => { - return [index, index]; - } -); - -const InfoFormat = ({ - datasetTitle, - singleValueCategories = new Map(singleValueCategoriesPlaceholder), - aboutURL = "thisisabouthtelengthofaurl", - skeleton = false, -}) => { - return ( -
-

{datasetTitle}

- {singleValueCategories.size > 0 && ( - <> -

- Dataset Metadata -

-
    - {Array.from(singleValueCategories).map((pair) => { - if (!pair[1] || pair[1] === "") return null; - return ( -
  • {`${pair[0]}: ${pair[1]}`}
    • - ); - })} -
- - )} - {aboutURL && ( - <> -

More Info

- - {aboutURL} - - - )} -
- ); -}; export default InfoDrawer; diff --git a/client/src/components/infoDrawer/infoFormat.js b/client/src/components/infoDrawer/infoFormat.js new file mode 100644 index 00000000..5a3db6a5 --- /dev/null +++ b/client/src/components/infoDrawer/infoFormat.js @@ -0,0 +1,194 @@ +import { H3, H1, UL, Classes } from "@blueprintjs/core"; +import React from "react"; + +const renderContributors = (contributors, affiliations, skeleton) => { + // eslint-disable-next-line no-constant-condition -- Temp removed contributor section to avoid publishing PII + if (contributors?.length === 0 && true) return null; + return ( + <> +

Contributors

+

+ {contributors.map((contributor) => { + const { email, name, institution } = contributor; + + return ( + + {name} + {email && `(${email})`} + {affiliations.indexOf(institution) + 1} + + ); + })} +

+ {renderAffiliations(affiliations, skeleton)} + + ); +}; + +const buildAffiliations = (contributors = []) => { + const affiliations = []; + contributors.forEach((contributor) => { + const { institution } = contributor; + if (affiliations.indexOf(institution) === -1) { + affiliations.push(institution); + } + }); + return affiliations; +}; + +const renderAffiliations = (affiliations, skeleton) => { + if (affiliations.length === 0) return null; + return ( + <> +

Affiliations

+
    + {affiliations.map((item, index) => ( +
    + {index + 1} + {" "} + {item} +
    + ))} +
+ + ); +}; + +const renderDOILink = (type, doi, skeleton) => { + if (!doi) return null; + return ( + doi && ( + <> +

{type}

+

+ + {doi} + +

+ + ) + ); +}; + +const renderOrganism = (organism, skeleton) => { + if (!organism) return null; + return ( + <> +

Organism

+

{organism}

+ + ); +}; + +const renderSingleValueCategories = (singleValueCategories, skeleton) => { + if (singleValueCategories.size === 0) return null; + return ( + <> +

Dataset Metadata

+
    + {Array.from(singleValueCategories).map((pair) => { + if (!pair[1] || pair[1] === "") return null; + return ( +
  • {`${pair[0]}: ${pair[1]}`}
    • + ); + })} +
+ + ); +}; + +const renderLinks = (projectLinks, aboutURL, skeleton) => { + if (!projectLinks && !aboutURL) return null; + if (projectLinks) + return ( + <> +

Project Links

+
    + {projectLinks.map((link) => { + if (link.link_type === "SUMMARY") return null; + return ( +
  • + + {link.link_name} + +
    • + ); + })} +
+ + ); + + return ( + <> +

More Info

+

+ + {aboutURL} + +

+ + ); +}; + +const NUM_CATEGORIES = 8; + +const singleValueCategoriesPlaceholder = Array.from(Array(NUM_CATEGORIES)).map( + (_, index) => { + return [index, index]; + } +); + +const InfoFormat = React.memo( + ({ + datasetTitle, + singleValueCategories = new Map(singleValueCategoriesPlaceholder), + aboutURL = "thisisabouthtelengthofaurl", + dataPortalProps = {}, + skeleton = false, + }) => { + if (dataPortalProps.corpora_schema_version === "1.0.0") { + dataPortalProps = {}; + } + const { + title, + publication_doi: doi, + preprint_doi: preprintDOI, + organism, + contributors, + project_links: projectLinks, + } = dataPortalProps; + + const affiliations = buildAffiliations(contributors); + + return ( +
+

+ {title ?? datasetTitle} +

+ {renderContributors(contributors, affiliations, skeleton)} + {renderDOILink("DOI", doi, skeleton)} + {renderDOILink("Preprint DOI", preprintDOI, skeleton)} + {renderOrganism(organism, skeleton)} + {renderSingleValueCategories(singleValueCategories, skeleton)} + {renderLinks(projectLinks, aboutURL, skeleton)} +
+ ); + } +); + +export default InfoFormat; diff --git a/client/src/components/termsPrompt/index.js b/client/src/components/termsPrompt/index.js index e99efe92..c6209a00 100644 --- a/client/src/components/termsPrompt/index.js +++ b/client/src/components/termsPrompt/index.js @@ -84,7 +84,7 @@ class TermsPrompt extends React.PureComponent { }} href={tosURL} target="_blank" - rel="noopener noreferrer" + rel="noopener" > terms of service @@ -106,7 +106,7 @@ class TermsPrompt extends React.PureComponent { }} href={privacyURL} target="_blank" - rel="noopener noreferrer" + rel="noopener" > privacy policy diff --git a/client/src/reducers/controls.js b/client/src/reducers/controls.js index e97038de..d5c4d406 100644 --- a/client/src/reducers/controls.js +++ b/client/src/reducers/controls.js @@ -21,7 +21,6 @@ const Controls = ( scatterplotYYaccessor: null, graphRenderCounter: 0 /* integer as Date: Fri, 11 Sep 2020 09:24:25 -0700 Subject: [PATCH 10/17] Change modify upgrade message to print to stderr instead of stdout (#1827) When generating a config file, you can do this: > cellxgene launch --dump-default-config > myconfig.yaml And then modify the myconfig.yaml. However, if an upgrade is available then you would get extra lines in the yaml file, which are not yaml code: There's a new version of cellxgene available (0.16.4)! To upgrade, run the following: pip install --upgrade cellxgene To solve this problem, the upgrade messages are sent to stderr instead, so they will appear on the screen and not in the config file. Alternatives: One workaround is "cellxgene --no-upgrade-check launch --dump-default-config > myconfig.yaml" But that's a bit verbose and not user friendly. The way we've setup the upgrade check to be separate and before the launch sub command, makes other code changes more involved. #1826 --- server/cli/upgrade.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/cli/upgrade.py b/server/cli/upgrade.py index 9ceda567..953f92ba 100644 --- a/server/cli/upgrade.py +++ b/server/cli/upgrade.py @@ -22,8 +22,8 @@ def log_upgrade_check(): release_tag_generator = (r["tag_name"] for r in _request_cellxgene_releases()) latest_release = next(release_tag_generator, lambda tag_name: validate_version_str(tag_name)) if version_gt(latest_release, __version__): - click.echo(f"There's a new version of cellxgene available ({latest_release})!") - click.echo("To upgrade, run the following: pip install --upgrade cellxgene\n") + click.echo(f"There's a new version of cellxgene available ({latest_release})!", err=True) + click.echo("To upgrade, run the following: pip install --upgrade cellxgene\n", err=True) except (ConnectionError, RateLimitException): click.echo("Upgrade check failed.\n") From a7a4580944aff4e93e6ae998cfeb5ae65a4fbf02 Mon Sep 17 00:00:00 2001 From: bmccandless Date: Fri, 11 Sep 2020 09:50:16 -0700 Subject: [PATCH 11/17] separate backend base url from frontend (#1819) * separate backend base url from frontend This is needed for auth, and to support a different location for the backend api server, than the frontend. part of chanzuckerberg/cellxgene#1778 new server config parameters: app__api_base_url, app__web_base_url Also changed api_base_url in the oauth config section to "oauth_api_base_url" to be less confusing with the app's api_base_url Other minor changes: changed how the jwt decode options are handled. Previously they needed to be set in a test case, and there was some extra logic to handle that. Now they are handled through comfig parameters, which makes it more general. Also, add a feature to set the CORS support credentials, which seems to be necessary for the backend/frontend separation, at least when run locally. This part is sort of experimental, and may be removed or changed later. --- server/app/app.py | 51 ++++++++++-- server/auth/auth_oauth.py | 78 +++++++++++-------- server/cli/launch.py | 2 +- server/common/app_config.py | 36 +++++++-- server/common/default_config.py | 32 ++++++-- server/test/__init__.py | 18 +++-- server/test/unit/auth/test_oauth.py | 56 ++++++------- server/test/unit/common/test_app_config.py | 91 +++++++++++++--------- 8 files changed, 243 insertions(+), 121 deletions(-) diff --git a/server/app/app.py b/server/app/app.py index 5f47e235..72378caf 100644 --- a/server/app/app.py +++ b/server/app/app.py @@ -2,6 +2,9 @@ import datetime import logging from functools import wraps from http import HTTPStatus +from urllib.parse import urlparse +import hashlib +import os from flask import Flask, redirect, current_app, make_response, render_template, abort, Blueprint, request, \ send_from_directory @@ -84,10 +87,12 @@ def dataset_index(url_dataroot=None, dataset=None): cache_manager = current_app.matrix_data_cache_manager with cache_manager.data_adaptor(url_dataroot, location, app_config) as data_adaptor: data_adaptor.set_uri_path(f"{url_dataroot}/{dataset}") - dataset_title = app_config.get_title(data_adaptor) - return render_template( - "index.html", datasetTitle=dataset_title, SCRIPTS=scripts, INLINE_SCRIPTS=inline_scripts - ) + args = { + "SCRIPTS" : scripts, + "INLINE_SCRIPTS" : inline_scripts + } + return render_template("index.html", **args) + except DatasetAccessError as e: return common_rest.abort_and_log( e.status_code, f"Invalid dataset {dataset}: {e.message}", loglevel=logging.INFO, include_exc_info=True @@ -179,9 +184,9 @@ def dataroot_test_index(): data += f"

Logged in as {auth.get_user_id()} / {auth.get_user_name()} / {auth.get_user_email()}

" if auth.requires_client_login(): if server_config.auth.is_user_authenticated(): - data += "

Logout

" + data += f"

Logout

" else: - data += "

Login

" + data += f"

Login

" datasets = [] for dataroot_dict in server_config.multi_dataset__dataroot.values(): @@ -329,6 +334,28 @@ def get_api_resources(bp_api, url_dataroot=None): return api +def handle_api_base_url(app, app_config): + """If an api_base_url is provided, then an inline script is generated to + handle the new API prefix""" + api_base_url = app_config.server_config.get_api_base_url() + if not api_base_url: + return + + if api_base_url.endswith("/"): + api_base_url = api_base_url[:-1] + + sha256 = hashlib.sha256(api_base_url.encode()).hexdigest() + script_name = f"api_base_url-{sha256}.js" + script_path = os.path.join(app.root_path, "../common/web/templates", script_name) + with open(script_path, "w") as fout: + fout.write("window.CELLXGENE.API.prefix = `" + api_base_url + "${location.pathname}api/`;\n") + + dataset_configs = [app_config.default_dataset_config] + list(app_config.dataroot_config.values()) + for dataset_config in dataset_configs: + inline_scripts = dataset_config.app__inline_scripts + inline_scripts.append(script_name) + + class Server: @staticmethod def _before_adding_routes(app, app_config): @@ -337,6 +364,7 @@ class Server: def __init__(self, app_config): self.app = Flask(__name__, static_folder=None) + handle_api_base_url(self.app, app_config) self._before_adding_routes(self.app, app_config) self.app.json_encoder = Float32JSONEncoder server_config = app_config.server_config @@ -353,6 +381,12 @@ class Server: self.app.register_blueprint(webbp) api_version = "/api/v0.2" + api_base_url = server_config.get_api_base_url() + api_path = "/" + if api_base_url: + parse = urlparse(api_base_url) + api_path = parse.path + if app_config.is_multi_dataset(): # NOTE: These routes only allow the dataset to be in the directory # of the dataroot, and not a subdirectory. We may want to change @@ -360,7 +394,8 @@ class Server: for dataroot_dict in server_config.multi_dataset__dataroot.values(): url_dataroot = dataroot_dict["base_url"] bp_api = Blueprint( - f"api_dataset_{url_dataroot}", __name__, url_prefix=f"/{url_dataroot}/" + api_version + f"api_dataset_{url_dataroot}", __name__, + url_prefix=f"{api_path}/{url_dataroot}/" + api_version ) resources = get_api_resources(bp_api, url_dataroot) self.app.register_blueprint(resources.blueprint) @@ -378,7 +413,7 @@ class Server: ) else: - bp_api = Blueprint("api", __name__, url_prefix=api_version) + bp_api = Blueprint("api", __name__, url_prefix=f"{api_path}{api_version}") resources = get_api_resources(bp_api) self.app.register_blueprint(resources.blueprint) self.app.add_url_rule( diff --git a/server/auth/auth_oauth.py b/server/auth/auth_oauth.py index 428d0048..2b4ddf45 100644 --- a/server/auth/auth_oauth.py +++ b/server/auth/auth_oauth.py @@ -1,7 +1,7 @@ from flask import session, request, redirect, current_app, after_this_request, has_request_context, g from server.auth.auth import AuthTypeClientBase, AuthTypeFactory from server.common.errors import AuthenticationError, ConfigurationError -from urllib.parse import urlencode +from urllib.parse import urlencode, urlparse import json import requests import base64 @@ -45,13 +45,20 @@ class AuthTypeOAuth(AuthTypeClientBase): if missingimport: raise ConfigurationError(f"oauth requires these modules: {', '.join(missingimport)}") self.algorithms = ["RS256"] - self.api_base_url = server_config.authentication__params_oauth__api_base_url + self.oauth_api_base_url = server_config.authentication__params_oauth__oauth_api_base_url self.client_id = server_config.authentication__params_oauth__client_id self.client_secret = server_config.authentication__params_oauth__client_secret - self.callback_base_url = server_config.authentication__params_oauth__callback_base_url self.session_cookie = server_config.authentication__params_oauth__session_cookie self.cookie_params = server_config.authentication__params_oauth__cookie + self.jwt_decode_options = server_config.authentication__params_oauth__jwt_decode_options + self._validate_cookie_params() + self._validate_jwt_decode_options() + + self.api_base_url = server_config.get_api_base_url() + self.web_base_url = server_config.get_web_base_url() + if self.api_base_url is None: + raise ConfigurationError("oauth requires the app__api_base_url to be set") # set the audience self.audience = self.client_id @@ -60,11 +67,13 @@ class AuthTypeOAuth(AuthTypeClientBase): # The JSON Web Key Set (JWKS) is a set of keys which contains the public keys used to verify # any JSON Web Token (JWT) issued by the authorization server and signed using the RS256 try: - jwksloc = f"{self.api_base_url}/.well-known/jwks.json" + jwksloc = f"{self.oauth_api_base_url}/.well-known/jwks.json" jwksurl = requests.get(jwksloc) self.jwks = jwksurl.json() except Exception: - raise ConfigurationError(f"error in oauth, api_url_base: {self.api_base_url}, cannot access {jwksloc}") + raise ConfigurationError( + f"error in oauth, api_url_base: {self.oauth_api_base_url}, cannot access {jwksloc}" + ) def _validate_cookie_params(self): """check the cookie_params, and raise a ConfigurationError if there is something wrong""" @@ -81,6 +90,20 @@ class AuthTypeOAuth(AuthTypeClientBase): if "key" not in keys: raise ConfigurationError("must have a key (name) in the cookie params") + def _validate_jwt_decode_options(self): + """check the jwt_decode_options, and raise a ConfigurationError if there is something wrong""" + if self.jwt_decode_options is None: + self.jwt_decode_options = {} + return + + valid_keys = { + "verify_signature", "verify_aud", "verify_iat", "verify_exp", "verify_nbf", "verify_iss", + "verify_sub", "verify_jti", "verify_at_hash", "leeway"} + keys = set(self.jwt_decode_options.keys()) + unknown = keys - valid_keys + if unknown: + raise ConfigurationError(f"unexpected key in jwt_decode_options: {', '.join(unknown)}") + def is_valid_authentication_type(self): return True @@ -88,27 +111,22 @@ class AuthTypeOAuth(AuthTypeClientBase): return True def add_url_rules(self, app): - app.add_url_rule("/login", "login", self.login, methods=["GET"]) - app.add_url_rule("/logout", "logout", self.logout, methods=["GET"]) - app.add_url_rule("/oauth2/callback", "callback", self.callback, methods=["GET"]) + parse = urlparse(self.api_base_url) + app.add_url_rule(f"{parse.path}/login", "login", self.login, methods=["GET"]) + app.add_url_rule(f"{parse.path}/logout", "logout", self.logout, methods=["GET"]) + app.add_url_rule(f"{parse.path}/oauth2/callback", "callback", self.callback, methods=["GET"]) def complete_setup(self, flask_app): self.oauth = OAuth(flask_app) - if self.callback_base_url is None: - # In this case, assume the server is running on the same host as the client, - # and the oauth provider has been configured - # with a callback that understands a localhost callback (e.g. A http://localhost:5005). - server_config = flask_app.app_config.server_config - self.callback_base_url = f"http://{server_config.app__host}:{server_config.app__port}" self.client = self.oauth.register( "auth0", client_id=self.client_id, client_secret=self.client_secret, - api_base_url=self.api_base_url, - refresh_token_url=f"{self.api_base_url}/oauth/token", - access_token_url=f"{self.api_base_url}/oauth/token", - authorize_url=f"{self.api_base_url}/authorize", + api_base_url=self.oauth_api_base_url, + refresh_token_url=f"{self.oauth_api_base_url}/oauth/token", + access_token_url=f"{self.oauth_api_base_url}/oauth/token", + authorize_url=f"{self.oauth_api_base_url}/authorize", client_kwargs={"scope": "openid profile email offline_access"}, ) @@ -138,9 +156,9 @@ class AuthTypeOAuth(AuthTypeClientBase): response.cache_control.update(dict(public=True, max_age=0, no_store=True, no_cache=True, must_revalidate=True)) def login(self): - callbackurl = f"{self.callback_base_url}/oauth2/callback" + callbackurl = f"{self.api_base_url}/oauth2/callback" return_path = request.args.get("dataset", "") - return_to = f"{self.callback_base_url}/{return_path}" + return_to = f"{self.web_base_url}/{return_path}/" # save the return path in the session cookie, accessed in the callback function session["oauth_callback_redirect"] = return_to response = self.client.authorize_redirect(redirect_uri=callbackurl) @@ -149,7 +167,7 @@ class AuthTypeOAuth(AuthTypeClientBase): def logout(self): self.remove_tokens() - params = {"returnTo": self.callback_base_url, "client_id": self.client_id} + params = {"returnTo": self.web_base_url, "client_id": self.client_id} response = redirect(self.client.api_base_url + "/v2/logout?" + urlencode(params)) self.update_response(response) return response @@ -228,14 +246,14 @@ class AuthTypeOAuth(AuthTypeClientBase): def get_login_url(self, data_adaptor): """Return the url for the login route""" - if current_app.app_config.is_multi_dataset(): - return f"/login?dataset={data_adaptor.uri_path}/" + if data_adaptor and current_app.app_config.is_multi_dataset(): + return f"{self.api_base_url}/login?dataset={data_adaptor.uri_path}/" else: - return "/login" + return f"{self.api_base_url}/login" def get_logout_url(self, data_adaptor): """Return the url for the logout route""" - return "/logout" + return f"{self.api_base_url}/logout" def check_jwt_payload(self, id_token): try: @@ -254,18 +272,14 @@ class AuthTypeOAuth(AuthTypeClientBase): "e": key.get("e"), } if rsa_key: - options = {} - if not rsa_key["n"] or not rsa_key["e"]: - # this is a mock auth server, do not validate - options = {"verify_signature": False, "verify_iss": False} try: payload = jwt.decode( id_token, rsa_key, algorithms=self.algorithms, audience=self.audience, - issuer=self.api_base_url + "/", - options=options, + issuer=self.oauth_api_base_url + "/", + options=self.jwt_decode_options, ) return payload @@ -321,7 +335,7 @@ class AuthTypeOAuth(AuthTypeClientBase): "client_secret": self.client_secret, } headers = {"content-type": "application/x-www-form-urlencoded"} - request = requests.post(f"{self.api_base_url}/oauth/token", urlencode(params), headers=headers) + request = requests.post(f"{self.oauth_api_base_url}/oauth/token", urlencode(params), headers=headers) if request.status_code != 200: # unable to refresh the token, log the user out self.remove_tokens() diff --git a/server/cli/launch.py b/server/cli/launch.py index d46161d4..d2a95f2a 100644 --- a/server/cli/launch.py +++ b/server/cli/launch.py @@ -296,7 +296,7 @@ class CliLaunchServer(Server): "application/octet-stream", ] Compress(app) - if app_config.server_config.app__debug: + if app_config.server_config.app__cors_supports_credentials or app_config.server_config.app__debug: CORS(app, supports_credentials=True) diff --git a/server/common/app_config.py b/server/common/app_config.py index ae67eeaf..487d70bb 100644 --- a/server/common/app_config.py +++ b/server/common/app_config.py @@ -417,6 +417,7 @@ class ServerConfig(BaseConfig): dictval_cases = [ ("app", "csp_directives"), ("authentication", "params_oauth", "cookie"), + ("authentication", "params_oauth", "jwt_decode_options"), ("adaptor", "cxg_adaptor", "tiledb_ctx"), ("multi_dataset", "dataroot"), ] @@ -434,13 +435,18 @@ class ServerConfig(BaseConfig): self.app__generate_cache_control_headers = dc["app"]["generate_cache_control_headers"] self.app__server_timing_headers = dc["app"]["server_timing_headers"] self.app__csp_directives = dc["app"]["csp_directives"] + self.app__cors_supports_credentials = dc["app"]["cors_supports_credentials"] + self.app__api_base_url = dc["app"]["api_base_url"] + self.app__web_base_url = dc["app"]["web_base_url"] self.authentication__type = dc["authentication"]["type"] - self.authentication__params_oauth__api_base_url = dc["authentication"]["params_oauth"]["api_base_url"] + self.authentication__params_oauth__oauth_api_base_url = dc["authentication"]["params_oauth"][ + "oauth_api_base_url" + ] self.authentication__params_oauth__client_id = dc["authentication"]["params_oauth"]["client_id"] self.authentication__params_oauth__client_secret = dc["authentication"]["params_oauth"]["client_secret"] - self.authentication__params_oauth__callback_base_url = \ - dc["authentication"]["params_oauth"]["callback_base_url"] + self.authentication__params_oauth__jwt_decode_options = dc["authentication"]["params_oauth"][ + "jwt_decode_options"] self.authentication__params_oauth__session_cookie = dc["authentication"]["params_oauth"]["session_cookie"] self.authentication__params_oauth__cookie = dc["authentication"]["params_oauth"]["cookie"] @@ -500,7 +506,10 @@ class ServerConfig(BaseConfig): self.check_attr("app__flask_secret_key", (type(None), str)) self.check_attr("app__generate_cache_control_headers", bool) self.check_attr("app__server_timing_headers", bool) + self.check_attr("app__cors_supports_credentials", bool) self.check_attr("app__csp_directives", (type(None), dict)) + self.check_attr("app__api_base_url", (type(None), str)) + self.check_attr("app__web_base_url", (type(None), str)) if self.app__port: try: @@ -549,15 +558,18 @@ class ServerConfig(BaseConfig): elif not isinstance(v, str): raise ConfigurationError("CSP directive value must be a string or list of strings.") + if self.app__web_base_url is None: + self.app__web_base_url = self.app__api_base_url + def handle_authentication(self, context): self.check_attr("authentication__type", (type(None), str)) # oauth ptypes = str if self.authentication__type == "oauth" else (type(None), str) - self.check_attr("authentication__params_oauth__api_base_url", ptypes) + self.check_attr("authentication__params_oauth__oauth_api_base_url", ptypes) self.check_attr("authentication__params_oauth__client_id", ptypes) self.check_attr("authentication__params_oauth__client_secret", ptypes) - self.check_attr("authentication__params_oauth__callback_base_url", (type(None), str)) + self.check_attr("authentication__params_oauth__jwt_decode_options", (type(None), dict)) self.check_attr("authentication__params_oauth__session_cookie", bool) if self.authentication__params_oauth__session_cookie: @@ -743,6 +755,18 @@ class ServerConfig(BaseConfig): return False return value > limit_value + def get_api_base_url(self): + if self.app__api_base_url == "local": + return f"http://{self.app__host}:{self.app__port}" + return self.app__api_base_url + + def get_web_base_url(self): + if self.app__web_base_url == "local": + return f"http://{self.app__host}:{self.app__port}" + if self.app__web_base_url is None: + return self.get_api_base_url() + return self.app__web_base_url + class DatasetConfig(BaseConfig): """Manages the config attribute associated with a dataset.""" @@ -769,7 +793,7 @@ class DatasetConfig(BaseConfig): self.user_annotations__ontology__obo_location = dc["user_annotations"]["ontology"]["obo_location"] self.user_annotations__hosted_tiledb_array__db_uri = dc["user_annotations"]["hosted_tiledb_array"]["db_uri"] self.user_annotations__hosted_tiledb_array__hosted_file_directory = \ - dc["user_annotations"]["hosted_tiledb_array"]["hosted_file_directory"] # noqa E501 + dc["user_annotations"][ "hosted_tiledb_array" ][ "hosted_file_directory" ] # noqa E501 self.embeddings__names = dc["embeddings"]["names"] self.embeddings__enable_reembedding = dc["embeddings"]["enable_reembedding"] diff --git a/server/common/default_config.py b/server/common/default_config.py index f473dd9f..3b58322f 100644 --- a/server/common/default_config.py +++ b/server/common/default_config.py @@ -14,6 +14,25 @@ server: server_timing_headers: false csp_directives: null + # CORS: Cross Origin Resource Sharing. If true, this allow users to make + # authenticated requests. This allows cookies and credentials to be submitted + # across domains + cors_supports_credentials: false + + # 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:"), 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: null + web_base_url: null + authentication: # The authentication types may be "none", "session", "oauth" # none: No authentication support, features like user_annotations must not be enabled. @@ -22,16 +41,17 @@ server: type: session params_oauth: - # url to the auth server - api_base_url: null + # url to the oauth server + oauth_api_base_url: null # client_id of this app client_id: null # the client_secret known to the auth server and this app client_secret: null - # cellxgene server location; - # the browser will be redirected to locations relative to this location during login and logout. - # A value of None, indicates the client and server are on the localhost. http://localhost: will be used. - callback_base_url: null + # jwt_decode_options, to specify non default decode options define + # jwt_decode_options to be a dictionary with key/values described by + # the options parameter of the jose.jwt.decode function: + # (https://python-jose.readthedocs.io/en/latest/jwt/api.html) + jwt_decode_options: null # if true, the jwt containing the id_token is stored in a session cookie session_cookie: true diff --git a/server/test/__init__.py b/server/test/__init__.py index 12159cc4..8ed6e3e7 100644 --- a/server/test/__init__.py +++ b/server/test/__init__.py @@ -131,16 +131,24 @@ def start_test_server(command_line_args=[], app_config=None): where the server can be accessed within the context, and is terminated when the context is exited. - The port is automatically set using find_available_port. + The port is automatically set using find_available_port, unless passed in as a command line arg. The verbose flag is automatically set to True. If an app_config is provided, then this function writes a temporary yaml config file, which this server will read and parse. """ - start = random.randint(DEFAULT_SERVER_PORT, 2 ** 16 - 1) - port = int(os.environ.get("CXG_SERVER_PORT", start)) - port = find_available_port("localhost", port) - command = ["cellxgene", "--no-upgrade-check", "launch", "--verbose", "--port=%d" % port] + command_line_args + command = ["cellxgene", "--no-upgrade-check", "launch", "--verbose"] + if "-p" in command_line_args: + port = int(command_line_args[command_line_args.index("-p") + 1]) + elif "--port" in command_line_args: + port = int(command_line_args[command_line_args.index("--port") + 1]) + else: + start = random.randint(DEFAULT_SERVER_PORT, 2 ** 16 - 1) + port = int(os.environ.get("CXG_SERVER_PORT", start)) + port = find_available_port("localhost", port) + command += ["--port=%d" % port] + + command += command_line_args tempdir = None if app_config: diff --git a/server/test/unit/auth/test_oauth.py b/server/test/unit/auth/test_oauth.py index 413a1084..42734a7b 100644 --- a/server/test/unit/auth/test_oauth.py +++ b/server/test/unit/auth/test_oauth.py @@ -19,7 +19,7 @@ from server.test import FIXTURES_ROOT, test_server # oauth server. # number of seconds that the oauth token is valid -TOKEN_EXPIRES = 5 +TOKEN_EXPIRES = 2 # Create a mocked out oauth token, which servers all the endpoints needed by the oauth type. mock_oauth_app = Flask("mock_oauth_app") @@ -34,17 +34,19 @@ def authorize(): @mock_oauth_app.route("/oauth/token", methods=["POST"]) def token(): + now = time.time() + expires_at = now + TOKEN_EXPIRES headers = dict(alg="RS256", kid="fake_kid") - payload = dict(name="fake_user", sub="fake_id", email="fake_user@email.com", email_verified=True) + payload = dict(name="fake_user", sub="fake_id", email="fake_user@email.com", email_verified=True, exp=expires_at) jwt = jose.jwt.encode(claims=payload, key="mysecret", algorithm="HS256", headers=headers) r = { - "access_token": f"access-{time.time()}", + "access_token": f"access-{now}", "id_token": jwt, - "refresh_token": f"random-{time.time()}", + "refresh_token": f"random-{now}", "scope": "openid profile email", "expires_in": TOKEN_EXPIRES, "token_type": "Bearer", - "expires_at": time.time() + TOKEN_EXPIRES, + "expires_at": expires_at } return make_response(jsonify(r)) @@ -81,6 +83,19 @@ class AuthTest(unittest.TestCase): def auth_flow(self, app_config, cookie_key=None): + app_config.update_server_config( + app__api_base_url="local", + authentication__type="oauth", + authentication__params_oauth__oauth_api_base_url=f"http://localhost:{PORT}", + authentication__params_oauth__client_id="mock_client_id", + authentication__params_oauth__client_secret="mock_client_secret", + authentication__params_oauth__jwt_decode_options={ + "verify_signature": False, "verify_iss": False + }) + + app_config.update_server_config(multi_dataset__dataroot=self.dataset_dataroot) + app_config.complete_config() + with test_server(app_config=app_config) as server: session = requests.Session() @@ -96,10 +111,10 @@ class AuthTest(unittest.TestCase): login_uri = config["config"]["authentication"]["login"] logout_uri = config["config"]["authentication"]["logout"] - self.assertEqual(login_uri, "/login?dataset=d/pbmc3k.cxg/") - self.assertEqual(logout_uri, "/logout") + self.assertEqual(login_uri, f"{server}/login?dataset=d/pbmc3k.cxg/") + self.assertEqual(logout_uri, f"{server}/logout") - r = session.get(f"{server}/{login_uri}") + r = session.get(login_uri) # check that the login redirect worked self.assertEqual(r.history[0].status_code, 302) self.assertEqual(r.url, f"{server}/d/pbmc3k.cxg/") @@ -113,13 +128,13 @@ class AuthTest(unittest.TestCase): cookie = session.cookies.get(cookie_key) token = json.loads(base64.b64decode(cookie)) access_token_before = token.get("access_token") - expires_at_before = token.get("expires_at") + id_token_before = token.get("id_token") # let the token expire time.sleep(TOKEN_EXPIRES + 1) # check that refresh works - session.get(f"{server}/{login_uri}") + session.get(login_uri) userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json() self.assertTrue(userinfo["userinfo"]["is_authenticated"]) self.assertEqual(userinfo["userinfo"]["username"], "fake_user") @@ -127,12 +142,12 @@ class AuthTest(unittest.TestCase): cookie = session.cookies.get(cookie_key) token = json.loads(base64.b64decode(cookie)) access_token_after = token.get("access_token") - expires_at_after = token.get("expires_at") + id_token_after = token.get("id_token") self.assertNotEqual(access_token_before, access_token_after) - self.assertTrue(expires_at_after - expires_at_before > TOKEN_EXPIRES) + self.assertNotEqual(id_token_before, id_token_after) - r = session.get(f"{server}/{logout_uri}") + r = session.get(logout_uri) # check that the logout redirect worked self.assertEqual(r.history[0].status_code, 302) self.assertEqual(r.url, f"{server}") @@ -146,31 +161,16 @@ class AuthTest(unittest.TestCase): # test with session cookies app_config = AppConfig() app_config.update_server_config( - authentication__type="oauth", - authentication__params_oauth__api_base_url=f"http://localhost:{PORT}", - authentication__params_oauth__client_id="mock_client_id", - authentication__params_oauth__client_secret="mock_client_secret", authentication__params_oauth__session_cookie=True, ) - - app_config.update_server_config(multi_dataset__dataroot=self.dataset_dataroot) - app_config.complete_config() - self.auth_flow(app_config) def test_auth_oauth_cookie(self): # test with specified cookie app_config = AppConfig() app_config.update_server_config( - authentication__type="oauth", - authentication__params_oauth__api_base_url=f"http://localhost:{PORT}", - authentication__params_oauth__client_id="mock_client_id", - authentication__params_oauth__client_secret="mock_client_secret", authentication__params_oauth__session_cookie=False, authentication__params_oauth__cookie=dict(key="test_cxguser", httponly=True, max_age=60), ) - app_config.update_server_config(multi_dataset__dataroot=self.dataset_dataroot) - app_config.complete_config() - self.auth_flow(app_config, "test_cxguser") diff --git a/server/test/unit/common/test_app_config.py b/server/test/unit/common/test_app_config.py index a299bf51..543fe6da 100644 --- a/server/test/unit/common/test_app_config.py +++ b/server/test/unit/common/test_app_config.py @@ -7,6 +7,7 @@ import requests from server.common.app_config import AppConfig from server.common.errors import ConfigurationError +from server.common.utils.utils import find_available_port from server.test import PROJECT_ROOT, test_server, FIXTURES_ROOT @@ -19,46 +20,46 @@ def mockenv(**envvars): class AppConfigTest(unittest.TestCase): def test_update(self): - c = AppConfig() - c.update_server_config(app__verbose=True, multi_dataset__dataroot="datadir") - v = c.server_config.changes_from_default() - self.assertCountEqual(v, [("app__verbose", True, False), ("multi_dataset__dataroot", "datadir", None)]) + config = AppConfig() + config.update_server_config(app__verbose=True, multi_dataset__dataroot="datadir") + vars = config.server_config.changes_from_default() + self.assertCountEqual(vars, [("app__verbose", True, False), ("multi_dataset__dataroot", "datadir", None)]) - c = AppConfig() - c.update_default_dataset_config(app__scripts=(), app__inline_scripts=()) - v = c.server_config.changes_from_default() - self.assertCountEqual(v, []) + config = AppConfig() + config.update_default_dataset_config(app__scripts=(), app__inline_scripts=()) + vars = config.server_config.changes_from_default() + self.assertCountEqual(vars, []) - c = AppConfig() - c.update_default_dataset_config(app__scripts=[], app__inline_scripts=[]) - v = c.default_dataset_config.changes_from_default() - self.assertCountEqual(v, []) + config = AppConfig() + config.update_default_dataset_config(app__scripts=[], app__inline_scripts=[]) + vars = config.default_dataset_config.changes_from_default() + self.assertCountEqual(vars, []) - c = AppConfig() - c.update_default_dataset_config(app__scripts=("a", "b"), app__inline_scripts=["c", "d"]) - v = c.default_dataset_config.changes_from_default() - self.assertCountEqual(v, [("app__scripts", ["a", "b"], []), ("app__inline_scripts", ["c", "d"], [])]) + config = AppConfig() + config.update_default_dataset_config(app__scripts=("a", "b"), app__inline_scripts=["c", "d"]) + vars = config.default_dataset_config.changes_from_default() + self.assertCountEqual(vars, [("app__scripts", ["a", "b"], []), ("app__inline_scripts", ["c", "d"], [])]) def test_multi_dataset(self): - c = AppConfig() + config = AppConfig() # test for illegal url_dataroots for illegal in ("../b", "!$*", "\\n", "", "(bad)"): - c.update_server_config( + config.update_server_config( multi_dataset__dataroot={"tag": {"base_url": illegal, "dataroot": "{PROJECT_ROOT}/example-dataset"}} ) with self.assertRaises(ConfigurationError): - c.complete_config() + config.complete_config() # test for legal url_dataroots for legal in ("d", "this.is-okay_", "a/b"): - c.update_server_config( + config.update_server_config( multi_dataset__dataroot={"tag": {"base_url": legal, "dataroot": "{PROJECT_ROOT}/example-dataset"}} ) - c.complete_config() + config.complete_config() # test that multi dataroots work end to end - c.update_server_config( + config.update_server_config( 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"), @@ -67,46 +68,46 @@ class AppConfigTest(unittest.TestCase): ) # Change this default to test if the dataroot overrides below work. - c.update_default_dataset_config(app__about_legal_tos="tos_default.html") + config.update_default_dataset_config(app__about_legal_tos="tos_default.html") # specialize the configs for set1 - c.add_dataroot_config( + config.add_dataroot_config( "s1", user_annotations__enable=False, diffexp__enable=True, app__about_legal_tos="tos_set1.html" ) # specialize the configs for set2 - c.add_dataroot_config( + config.add_dataroot_config( "s2", user_annotations__enable=True, diffexp__enable=False, app__about_legal_tos="tos_set2.html" ) # no specializations for set3 (they get the default dataset config) - c.complete_config() + config.complete_config() - with test_server(app_config=c) as server: + with test_server(app_config=config) as server: session = requests.Session() - r = session.get(f"{server}/set1/1/2/pbmc3k.h5ad/api/v0.2/config") - data_config = r.json() + response = session.get(f"{server}/set1/1/2/pbmc3k.h5ad/api/v0.2/config") + data_config = response.json() assert data_config["config"]["displayNames"]["dataset"] == "pbmc3k" assert data_config["config"]["parameters"]["annotations"] is False assert data_config["config"]["parameters"]["disable-diffexp"] is False assert data_config["config"]["parameters"]["about_legal_tos"] == "tos_set1.html" - r = session.get(f"{server}/set2/pbmc3k.cxg/api/v0.2/config") - data_config = r.json() + response = session.get(f"{server}/set2/pbmc3k.cxg/api/v0.2/config") + data_config = response.json() assert data_config["config"]["displayNames"]["dataset"] == "pbmc3k" assert data_config["config"]["parameters"]["annotations"] is True assert data_config["config"]["parameters"]["about_legal_tos"] == "tos_set2.html" - r = session.get(f"{server}/set3/pbmc3k.cxg/api/v0.2/config") - data_config = r.json() + response = session.get(f"{server}/set3/pbmc3k.cxg/api/v0.2/config") + data_config = response.json() assert data_config["config"]["displayNames"]["dataset"] == "pbmc3k" assert data_config["config"]["parameters"]["annotations"] is True assert data_config["config"]["parameters"]["disable-diffexp"] is False assert data_config["config"]["parameters"]["about_legal_tos"] == "tos_default.html" - r = session.get(f"{server}/health") - assert r.json()["status"] == "pass" + response = session.get(f"{server}/health") + assert response.json()["status"] == "pass" @mockenv(CXG_AWS_SECRET_NAME="TESTING", CXG_AWS_SECRET_REGION_NAME="TEST_REGION") @patch('server.common.aws_secret_utils.get_secret_key') @@ -133,3 +134,23 @@ class AppConfigTest(unittest.TestCase): 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") + + def test_api_base_url(self): + + # test the api_base_url feature, and that it can contain a path + config = AppConfig() + backend_port = find_available_port("localhost", 10000) + config.update_server_config( + app__api_base_url=f"http://localhost:{backend_port}/additional/path/before/dataroot", + multi_dataset__dataroot=f"{PROJECT_ROOT}/example-dataset" + ) + + config.complete_config() + + with test_server(["-p", str(backend_port)], app_config=config) as server: + session = requests.Session() + self.assertEqual(server, f"http://localhost:{backend_port}") + response = session.get(f"{server}/additional/path/before/dataroot/d/pbmc3k.h5ad/api/v0.2/config") + self.assertEqual(response.status_code, 200) + data_config = response.json() + self.assertEqual(data_config["config"]["displayNames"]["dataset"], "pbmc3k") From 4b240920e242630ef85da0dd4cac788f87f40f96 Mon Sep 17 00:00:00 2001 From: maniarathi Date: Sat, 12 Sep 2020 10:32:00 -0700 Subject: [PATCH 12/17] Pass in the previous crossfilter when creating a new annomatrix for a switched embedding in order to retain the previous selection of cells. (#1832) * Pass in the previous crossfilter when creating a new annomatrix for a switched embedding in order to retain the previous selection of cells. * Address Bruce's PR comment --- client/src/actions/embedding.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/client/src/actions/embedding.js b/client/src/actions/embedding.js index 5e882160..615bcd68 100644 --- a/client/src/actions/embedding.js +++ b/client/src/actions/embedding.js @@ -5,14 +5,14 @@ action creators related to embeddings choice import { AnnoMatrixObsCrossfilter } from "../annoMatrix"; import { _setEmbeddingSubset } from "../util/stateManager/viewStackHelpers"; -export async function _switchEmbedding(prevAnnoMatrix, newEmbeddingName) { +export async function _switchEmbedding(prevAnnoMatrix, prevCrossfilter, newEmbeddingName) { /* DRY helper used by this and reembedding action creators */ const base = prevAnnoMatrix.base(); const embeddingDf = await base.fetch("emb", newEmbeddingName); const annoMatrix = _setEmbeddingSubset(prevAnnoMatrix, embeddingDf); - const obsCrossfilter = await new AnnoMatrixObsCrossfilter(annoMatrix).select( + const obsCrossfilter = await new AnnoMatrixObsCrossfilter(annoMatrix, prevCrossfilter.obsCrossfilter).select( "emb", newEmbeddingName, { @@ -30,9 +30,10 @@ export const layoutChoiceAction = (newLayoutChoice) => async ( On layout choice, make sure we have selected all on the previous layout, AND the new layout. */ - const { annoMatrix: prevAnnoMatrix } = getState(); + const { annoMatrix: prevAnnoMatrix, obsCrossfilter: prevCrossfilter } = getState(); const [annoMatrix, obsCrossfilter] = await _switchEmbedding( prevAnnoMatrix, + prevCrossfilter, newLayoutChoice ); dispatch({ From 6a7ae8bc8e1978f39651d7de4082e4423a75f064 Mon Sep 17 00:00:00 2001 From: bmccandless Date: Sat, 12 Sep 2020 10:56:31 -0700 Subject: [PATCH 13/17] Fixes from frontend/backend url separation (#1829) * Fixes from frontend/backend url separation This fixes the CORS and CSP headers. Also, in thie commit, I removed the cors_supports_credentials config parameter, which was recently introduced. Instead, the logic determines the need to use CORS headers if the web_page_url is set. #1778 --- server/app/app.py | 3 --- server/cli/launch.py | 2 +- server/common/app_config.py | 8 +++++--- server/common/default_config.py | 5 ----- server/eb/app.py | 20 ++++++++++++++++++-- 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/server/app/app.py b/server/app/app.py index 72378caf..3b48d6ea 100644 --- a/server/app/app.py +++ b/server/app/app.py @@ -341,9 +341,6 @@ def handle_api_base_url(app, app_config): if not api_base_url: return - if api_base_url.endswith("/"): - api_base_url = api_base_url[:-1] - sha256 = hashlib.sha256(api_base_url.encode()).hexdigest() script_name = f"api_base_url-{sha256}.js" script_path = os.path.join(app.root_path, "../common/web/templates", script_name) diff --git a/server/cli/launch.py b/server/cli/launch.py index d2a95f2a..d46161d4 100644 --- a/server/cli/launch.py +++ b/server/cli/launch.py @@ -296,7 +296,7 @@ class CliLaunchServer(Server): "application/octet-stream", ] Compress(app) - if app_config.server_config.app__cors_supports_credentials or app_config.server_config.app__debug: + if app_config.server_config.app__debug: CORS(app, supports_credentials=True) diff --git a/server/common/app_config.py b/server/common/app_config.py index 487d70bb..3caccec5 100644 --- a/server/common/app_config.py +++ b/server/common/app_config.py @@ -435,7 +435,6 @@ class ServerConfig(BaseConfig): self.app__generate_cache_control_headers = dc["app"]["generate_cache_control_headers"] self.app__server_timing_headers = dc["app"]["server_timing_headers"] self.app__csp_directives = dc["app"]["csp_directives"] - self.app__cors_supports_credentials = dc["app"]["cors_supports_credentials"] self.app__api_base_url = dc["app"]["api_base_url"] self.app__web_base_url = dc["app"]["web_base_url"] @@ -506,7 +505,6 @@ class ServerConfig(BaseConfig): self.check_attr("app__flask_secret_key", (type(None), str)) self.check_attr("app__generate_cache_control_headers", bool) self.check_attr("app__server_timing_headers", bool) - self.check_attr("app__cors_supports_credentials", bool) self.check_attr("app__csp_directives", (type(None), dict)) self.check_attr("app__api_base_url", (type(None), str)) self.check_attr("app__web_base_url", (type(None), str)) @@ -758,6 +756,8 @@ class ServerConfig(BaseConfig): def get_api_base_url(self): if self.app__api_base_url == "local": return f"http://{self.app__host}:{self.app__port}" + if self.app__api_base_url and self.app__api_base_url.endswith("/"): + return self.app__api_base_url[:-1] return self.app__api_base_url def get_web_base_url(self): @@ -765,7 +765,9 @@ class ServerConfig(BaseConfig): return f"http://{self.app__host}:{self.app__port}" if self.app__web_base_url is None: return self.get_api_base_url() - return self.app__web_base_url + if self.app__web_base_url.endswith("/"): + return self.app__web_base_url[:-1] + return self.api__web_base_url class DatasetConfig(BaseConfig): diff --git a/server/common/default_config.py b/server/common/default_config.py index 3b58322f..16e0c27d 100644 --- a/server/common/default_config.py +++ b/server/common/default_config.py @@ -14,11 +14,6 @@ server: server_timing_headers: false csp_directives: null - # CORS: Cross Origin Resource Sharing. If true, this allow users to make - # authenticated requests. This allows cookies and credentials to be submitted - # across domains - cors_supports_credentials: false - # 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: diff --git a/server/eb/app.py b/server/eb/app.py index 159e6dc4..7a5e834c 100644 --- a/server/eb/app.py +++ b/server/eb/app.py @@ -4,10 +4,11 @@ import sys import os import hashlib import base64 +from urllib.parse import urlparse from flask import json import logging from flask_talisman import Talisman - +from flask_cors import CORS from server.common.aws_secret_utils import handle_config_from_secret from server.common.errors import SecretKeyRetrievalError @@ -41,6 +42,14 @@ class WSGIServer(Server): def _before_adding_routes(app, app_config): script_hashes = WSGIServer.get_csp_hashes(app, app_config) server_config = app_config.server_config + + # add the api_base_url to the connect_src csp header. + extra_connect_src = [] + api_base_url = server_config.get_api_base_url() + if api_base_url: + parse_api_base_url = urlparse(api_base_url) + extra_connect_src = [f"{parse_api_base_url.scheme}://{parse_api_base_url.netloc}"] + # This hash should be in sync with the script within # `client/configuration/webpack/obsoleteHTMLTemplate.html` @@ -51,7 +60,7 @@ class WSGIServer(Server): obsolete_browser_script_hash = ["'sha256-/rmgOi/skq9MpiZxPv6lPb1PNSN+Uf4NaUHO/IjyfwM='"] csp = { "default-src": ["'self'"], - "connect-src": ["'self'"], + "connect-src": ["'self'"] + extra_connect_src, "script-src": ["'self'", "'unsafe-eval'"] + obsolete_browser_script_hash + script_hashes, "style-src": ["'self'", "'unsafe-inline'"], @@ -70,6 +79,13 @@ class WSGIServer(Server): v = [v] csp[k] = csp.get(k, []) + v + # Add the web_base_url to the CORS header + web_base_url = server_config.get_web_base_url() + if web_base_url: + web_base_url_parse = urlparse(web_base_url) + allowed_origin = f"{web_base_url_parse.scheme}://{web_base_url_parse.netloc}" + CORS(app, supports_credentials=True, origins=allowed_origin) + Talisman( app, force_https=server_config.app__force_https, frame_options="DENY", content_security_policy=csp, ) From 342a9d774c93730ab79f5000a87bd9ddd4a2fa37 Mon Sep 17 00:00:00 2001 From: bmccandless Date: Mon, 14 Sep 2020 13:15:47 -0700 Subject: [PATCH 14/17] app config bug fix: (#1833) * app config bug fix: When reading a config file that included per_dataset_config, the dataroot specializations were applied, but not the default config. This PR fixes that and also includes a test for this case. --- server/common/app_config.py | 5 ++- server/test/unit/common/test_app_config.py | 42 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/server/common/app_config.py b/server/common/app_config.py index 3caccec5..e0a49e46 100644 --- a/server/common/app_config.py +++ b/server/common/app_config.py @@ -100,7 +100,10 @@ class AppConfig(object): per_dataset_config = config.get("per_dataset_config", {}) for key, dataroot_config in per_dataset_config.items(): - self.add_dataroot_config(key, **dataroot_config) + # first create and initialize the dataroot with the default config + self.add_dataroot_config(key, **config["dataset"]) + # then apply the per dataset configuration + self.dataroot_config[key].update_from_config(dataroot_config, f"per_dataset_config__{key}") self.is_complete = False diff --git a/server/test/unit/common/test_app_config.py b/server/test/unit/common/test_app_config.py index 543fe6da..f8a20093 100644 --- a/server/test/unit/common/test_app_config.py +++ b/server/test/unit/common/test_app_config.py @@ -2,6 +2,7 @@ import os import unittest from unittest import mock from unittest.mock import patch +import tempfile import requests @@ -154,3 +155,44 @@ class AppConfigTest(unittest.TestCase): self.assertEqual(response.status_code, 200) data_config = response.json() self.assertEqual(data_config["config"]["displayNames"]["dataset"], "pbmc3k") + + def test_configfile_with_specialization(self): + # test that per_dataset_config config load the default config, then the specialized config + + with tempfile.TemporaryDirectory() as tempdir: + configfile = os.path.join(tempdir, "config.yaml") + with open(configfile, "w") as fconfig: + config = """ + server: + multi_dataset: + dataroot: + test: + base_url: test + dataroot: fake_dataroot + + dataset: + user_annotations: + enable: false + type: hosted_tiledb_array + hosted_tiledb_array: + db_uri: fake_db_uri + hosted_file_directory: fake_dir + + per_dataset_config: + test: + user_annotations: + enable: true + """ + fconfig.write(config) + + app_config = AppConfig() + app_config.update_from_config_file(configfile) + + test_config = app_config.dataroot_config["test"] + + # test config from default + self.assertEqual(test_config.user_annotations__type, "hosted_tiledb_array") + self.assertEqual(test_config.user_annotations__hosted_tiledb_array__db_uri, "fake_db_uri") + + # test config from specialization + self.assertTrue(test_config.user_annotations__enable) From 9fac6849a305bccd84c71475d6f4090df0a833ad Mon Sep 17 00:00:00 2001 From: maniarathi Date: Mon, 14 Sep 2020 17:19:24 -0700 Subject: [PATCH 15/17] Fix import of anndata from master so that there aren't issues with scanpy version checking. (#1834) --- .github/workflows/compatibility_tests.yml | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/.github/workflows/compatibility_tests.yml b/.github/workflows/compatibility_tests.yml index 11c9d0cd..bd3c41e8 100644 --- a/.github/workflows/compatibility_tests.yml +++ b/.github/workflows/compatibility_tests.yml @@ -67,11 +67,6 @@ jobs: uses: actions/checkout@v2 with: path: cellxgene - - name: Checkout tools repo - uses: actions/checkout@v2 - with: - repository: theislab/anndata - path: anndata - name: Install dependencies run: | cd cellxgene @@ -82,7 +77,7 @@ jobs: # 2. install cellxgene pip install --upgrade cellxgene # 3. install anndata - cd ../anndata && pip install -e . + pip install git+https://github.com/theislab/anndata - name: Tests run: cd cellxgene && make unit-test ${{ matrix.test-suite }} @@ -102,17 +97,11 @@ jobs: uses: actions/checkout@v2 with: path: cellxgene - - name: Checkout tools repo - uses: actions/checkout@v2 - with: - repository: theislab/anndata - path: anndata - name: Install dependencies run: | cd cellxgene sed -i -E 's/^anndata[>=]=[0-9]+.[0-9]+.[0-9]+$/anndata/g' server/requirements.txt make pydist install-dist dev-env - cd ../anndata - pip install -e . + pip install git+https://github.com/theislab/anndata - name: Tests run: cd cellxgene && make unit-test ${{ matrix.test-suite }} From 4f339e89b1d3fe870ae069f56d245ff92b1795da Mon Sep 17 00:00:00 2001 From: Madison Dunitz Date: Tue, 15 Sep 2020 11:20:00 -0500 Subject: [PATCH 16/17] dont cache schema (#1836) --- server/app/app.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/app/app.py b/server/app/app.py index 3b48d6ea..d926f515 100644 --- a/server/app/app.py +++ b/server/app/app.py @@ -233,7 +233,8 @@ class DatasetResource(Resource): class SchemaAPI(DatasetResource): - @cache_control(public=True, max_age=ONE_WEEK) + # TODO @mdunitz separate dataset schema and user schema + @cache_control(no_store=True) @rest_get_data_adaptor def get(self, data_adaptor): return common_rest.schema_get(data_adaptor) From 3e9cb0265e045e7f609620db4c0a185a03333058 Mon Sep 17 00:00:00 2001 From: Severiano Badajoz Date: Tue, 15 Sep 2020 12:03:59 -0700 Subject: [PATCH 17/17] Fix InfoFormat parameter checking (#1831) Went through and ensured that undefined/null values were caught and handled correctly in render functions. Also documented some of the more complicated functions. --- Closes #1825 --- .../src/components/infoDrawer/infoFormat.js | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/client/src/components/infoDrawer/infoFormat.js b/client/src/components/infoDrawer/infoFormat.js index 5a3db6a5..8361b6de 100644 --- a/client/src/components/infoDrawer/infoFormat.js +++ b/client/src/components/infoDrawer/infoFormat.js @@ -3,7 +3,7 @@ import React from "react"; const renderContributors = (contributors, affiliations, skeleton) => { // eslint-disable-next-line no-constant-condition -- Temp removed contributor section to avoid publishing PII - if (contributors?.length === 0 && true) return null; + if (!contributors || contributors.length === 0 || true) return null; return ( <>

Contributors

@@ -25,6 +25,7 @@ const renderContributors = (contributors, affiliations, skeleton) => { ); }; +// generates a list of unique institutions by order of appearance in contributors const buildAffiliations = (contributors = []) => { const affiliations = []; contributors.forEach((contributor) => { @@ -43,11 +44,7 @@ const renderAffiliations = (affiliations, skeleton) => {

Affiliations

    {affiliations.map((item, index) => ( -
    +
    {index + 1} {" "} {item} @@ -61,16 +58,14 @@ const renderAffiliations = (affiliations, skeleton) => { const renderDOILink = (type, doi, skeleton) => { if (!doi) return null; return ( - doi && ( - <> -

    {type}

    -

    - - {doi} - -

    - - ) + <> +

    {type}

    +

    + + {doi} + +

    + ); }; @@ -84,6 +79,8 @@ const renderOrganism = (organism, skeleton) => { ); }; +// Render list of metadata attributes found in categorical field +// Ignores categories with empty or null values const renderSingleValueCategories = (singleValueCategories, skeleton) => { if (singleValueCategories.size === 0) return null; return ( @@ -104,6 +101,8 @@ 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) => { if (!projectLinks && !aboutURL) return null; if (projectLinks) @@ -147,6 +146,7 @@ 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];