From 7e12eb3cb08365a2787b6dae8f192b221d538098 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 13 Dec 2023 16:48:56 -0500 Subject: [PATCH 01/23] style: black reformat --- nmdc_runtime/api/endpoints/queries.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/nmdc_runtime/api/endpoints/queries.py b/nmdc_runtime/api/endpoints/queries.py index 4a272797..bc311d0f 100644 --- a/nmdc_runtime/api/endpoints/queries.py +++ b/nmdc_runtime/api/endpoints/queries.py @@ -34,7 +34,7 @@ def check_can_update_and_delete(user: User): if not permitted(user.username, "/queries:run(query_cmd:DeleteCommand)"): raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, - detail="Only specific users are allowed to issue update and delete commands." + detail="Only specific users are allowed to issue update and delete commands.", ) @@ -125,7 +125,8 @@ def _run_query(query, mdb) -> CommandResponse: detail="Can only delete documents in nmdc-schema collections.", ) delete_specs = [ - {"filter": del_statement.q, "limit": del_statement.limit} for del_statement in query.cmd.deletes + {"filter": del_statement.q, "limit": del_statement.limit} + for del_statement in query.cmd.deletes ] for spec in delete_specs: docs = list(mdb[collection_name].find(**spec)) @@ -148,7 +149,8 @@ def _run_query(query, mdb) -> CommandResponse: detail="Can only update documents in nmdc-schema collections.", ) update_specs = [ - {"filter": up_statement.q, "limit": 0 if up_statement.multi else 1} for up_statement in query.cmd.updates + {"filter": up_statement.q, "limit": 0 if up_statement.multi else 1} + for up_statement in query.cmd.updates ] for spec in update_specs: docs = list(mdb[collection_name].find(**spec)) From 0e39efa2de2f63d0674ddc06e435df05a9946300 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 13 Dec 2023 16:51:02 -0500 Subject: [PATCH 02/23] fix: unique index on username for users collection closes #325 --- nmdc_runtime/api/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nmdc_runtime/api/main.py b/nmdc_runtime/api/main.py index 71a6863e..6db86c98 100644 --- a/nmdc_runtime/api/main.py +++ b/nmdc_runtime/api/main.py @@ -249,7 +249,7 @@ def ensure_initial_resources_on_boot(): ).model_dump(exclude_unset=True), upsert=True, ) - mdb.users.create_index("username") + mdb.users.create_index("username", unique=True) site_id = os.getenv("API_SITE_ID") runtime_site_ok = mdb.sites.count_documents(({"id": site_id})) > 0 From 93411e5806add40d269f45f7fa7b09deedba00d8 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 13 Dec 2023 17:14:56 -0500 Subject: [PATCH 03/23] fix: permission for /metadata/json:submit needed given that anyone can create an account via ORCiD. --- nmdc_runtime/api/endpoints/metadata.py | 5 +++++ nmdc_runtime/api/main.py | 10 +++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/nmdc_runtime/api/endpoints/metadata.py b/nmdc_runtime/api/endpoints/metadata.py index 112d4098..810c6b56 100644 --- a/nmdc_runtime/api/endpoints/metadata.py +++ b/nmdc_runtime/api/endpoints/metadata.py @@ -239,6 +239,11 @@ async def submit_json_nmdcdb( Submit a NMDC JSON Schema "nmdc:Database" object. """ + if not permitted(user.username, "/metadata/json:submit"): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Only specific users are allowed to submit json at this time.", + ) rv = validate_json(docs, mdb) if rv["result"] == "errors": raise HTTPException( diff --git a/nmdc_runtime/api/main.py b/nmdc_runtime/api/main.py index 6db86c98..e8fe4015 100644 --- a/nmdc_runtime/api/main.py +++ b/nmdc_runtime/api/main.py @@ -302,16 +302,12 @@ def ensure_default_api_perms(): allowed = { "/metadata/changesheets:submit": [ "admin", - "dwinston", - "mam", - "montana", - "pajau", - "spatil", ], "/queries:run(query_cmd:DeleteCommand)": [ "admin", - "dwinston", - "scanon", + ], + "/metadata/json:submit": [ + "admin", ], } for doc in [ From 66652ecf3e7e1ce9293fd562545d346a0ca6ee09 Mon Sep 17 00:00:00 2001 From: eecavanna Date: Fri, 15 Dec 2023 11:52:59 -0800 Subject: [PATCH 04/23] Update `nmdc-schema` package from `9.2.0` to `9.3.1` --- requirements/dev.txt | 64 ++++++++++++++----------- requirements/main.in | 2 +- requirements/main.txt | 108 ++++++++++++++++++++++++------------------ 3 files changed, 99 insertions(+), 75 deletions(-) diff --git a/requirements/dev.txt b/requirements/dev.txt index aca7b8cb..390e5a21 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -4,18 +4,31 @@ # # pip-compile --allow-unsafe --output-file=requirements/dev.txt --strip-extras requirements/dev.in # +aiohttp==3.9.1 + # via + # -c requirements/main.txt + # black +aiosignal==1.3.1 + # via + # -c requirements/main.txt + # aiohttp +async-timeout==4.0.3 + # via + # -c requirements/main.txt + # aiohttp attrs==23.1.0 # via # -c requirements/main.txt + # aiohttp # cattrs # requests-cache -black==23.11.0 +black==23.12.0 # via # -c requirements/main.txt # -r requirements/dev.in build==1.0.3 # via pip-tools -cattrs==23.2.2 +cattrs==23.2.3 # via # -c requirements/main.txt # requests-cache @@ -23,10 +36,6 @@ certifi==2023.11.17 # via # -c requirements/main.txt # requests -cffi==1.16.0 - # via - # -c requirements/main.txt - # cryptography charset-normalizer==3.3.2 # via # -c requirements/main.txt @@ -36,14 +45,10 @@ click==8.1.7 # -c requirements/main.txt # black # pip-tools -coverage==7.3.2 +coverage==7.3.3 # via # -r requirements/dev.in # pytest-cov -cryptography==41.0.7 - # via - # -c requirements/main.txt - # secretstorage docutils==0.20.1 # via # -c requirements/main.txt @@ -55,11 +60,17 @@ exceptiongroup==1.2.0 # pytest flake8==6.1.0 # via -r requirements/dev.in +frozenlist==1.4.1 + # via + # -c requirements/main.txt + # aiohttp + # aiosignal idna==3.6 # via # -c requirements/main.txt # requests -importlib-metadata==6.8.0 + # yarl +importlib-metadata==7.0.0 # via # keyring # twine @@ -71,10 +82,6 @@ invoke==2.2.0 # via -r requirements/dev.in jaraco-classes==3.3.0 # via keyring -jeepney==0.8.0 - # via - # keyring - # secretstorage keyring==24.3.0 # via twine markdown-it-py==3.0.0 @@ -89,11 +96,16 @@ mdurl==0.1.2 # markdown-it-py more-itertools==10.1.0 # via jaraco-classes +multidict==6.0.4 + # via + # -c requirements/main.txt + # aiohttp + # yarl mypy-extensions==1.0.0 # via # -c requirements/main.txt # black -nh3==0.2.14 +nh3==0.2.15 # via readme-renderer packaging==23.2 # via @@ -101,7 +113,7 @@ packaging==23.2 # black # build # pytest -pathspec==0.11.2 +pathspec==0.12.1 # via # -c requirements/main.txt # black @@ -109,7 +121,7 @@ pip-tools==7.3.0 # via -r requirements/dev.in pkginfo==1.9.6 # via twine -platformdirs==4.0.0 +platformdirs==4.1.0 # via # -c requirements/main.txt # black @@ -120,10 +132,6 @@ pluggy==1.3.0 # pytest pycodestyle==2.11.1 # via flake8 -pycparser==2.21 - # via - # -c requirements/main.txt - # cffi pyflakes==3.1.0 # via # -c requirements/main.txt @@ -141,7 +149,7 @@ pytest==7.4.3 # -r requirements/dev.in # pytest-asyncio # pytest-cov -pytest-asyncio==0.21.1 +pytest-asyncio==0.23.2 # via -r requirements/dev.in pytest-cov==4.1.0 # via -r requirements/dev.in @@ -168,8 +176,6 @@ rfc3986==2.0.0 # via twine rich==13.7.0 # via twine -secretstorage==3.3.3 - # via keyring six==1.16.0 # via # -c requirements/main.txt @@ -186,7 +192,7 @@ tomli==2.0.1 # pytest twine==4.0.2 # via -r requirements/dev.in -typing-extensions==4.8.0 +typing-extensions==4.9.0 # via # -c requirements/main.txt # black @@ -203,6 +209,10 @@ urllib3==1.26.18 # twine wheel==0.42.0 # via pip-tools +yarl==1.9.4 + # via + # -c requirements/main.txt + # aiohttp zipp==3.17.0 # via importlib-metadata diff --git a/requirements/main.in b/requirements/main.in index 7d4db727..7fdc1e67 100644 --- a/requirements/main.in +++ b/requirements/main.in @@ -24,7 +24,7 @@ mkdocs-jupyter mkdocs-material mkdocs-mermaid2-plugin motor -nmdc-schema==9.2.0 +nmdc-schema==9.3.1 openpyxl pandas passlib[bcrypt] diff --git a/requirements/main.txt b/requirements/main.txt index 852180fb..83caa909 100644 --- a/requirements/main.txt +++ b/requirements/main.txt @@ -4,9 +4,13 @@ # # pip-compile --allow-unsafe --output-file=requirements/main.txt --strip-extras requirements/main.in # +aiohttp==3.9.1 + # via black +aiosignal==1.3.1 + # via aiohttp alabaster==0.7.13 # via sphinx -alembic==1.12.1 +alembic==1.13.0 # via dagster aniso8601==9.0.1 # via graphene @@ -23,6 +27,8 @@ anyio==3.7.1 # jupyter-server # starlette # watchfiles +appnope==0.1.3 + # via ipykernel argon2-cffi==23.1.0 # via jupyter-server argon2-cffi-bindings==21.2.0 @@ -33,15 +39,18 @@ asttokens==2.4.1 # via stack-data async-lru==2.0.4 # via jupyterlab +async-timeout==4.0.3 + # via aiohttp attrs==23.1.0 # via + # aiohttp # cattrs # jsonschema # referencing # requests-cache autoflake==2.2.1 # via shed -babel==2.13.1 +babel==2.14.0 # via # jupyterlab-server # mkdocs-material @@ -50,7 +59,7 @@ backoff==2.2.1 # via gql base32-lib==1.0.2 # via -r requirements/main.in -bcrypt==4.1.1 +bcrypt==4.1.2 # via passlib beanie==1.23.6 # via -r requirements/main.in @@ -59,19 +68,19 @@ beautifulsoup4==4.12.2 # -r requirements/main.in # mkdocs-mermaid2-plugin # nbconvert -black==23.11.0 +black==23.12.0 # via shed bleach==6.1.0 # via nbconvert -boto3==1.33.3 +boto3==1.34.1 # via -r requirements/main.in -botocore==1.33.3 +botocore==1.34.1 # via # boto3 # s3transfer cachetools==5.3.2 # via tox -cattrs==23.2.2 +cattrs==23.2.3 # via requests-cache certifi==2023.11.17 # via requests @@ -126,23 +135,23 @@ curies==0.7.4 # via # linkml-runtime # prefixmaps -dagit==1.5.9 +dagit==1.5.13 # via -r requirements/main.in -dagster==1.5.9 +dagster==1.5.13 # via # -r requirements/main.in # dagster-graphql # dagster-postgres # dagster-webserver -dagster-graphql==1.5.9 +dagster-graphql==1.5.13 # via # -r requirements/main.in # dagster-webserver -dagster-pipes==1.5.9 +dagster-pipes==1.5.13 # via dagster -dagster-postgres==0.21.9 +dagster-postgres==0.21.13 # via -r requirements/main.in -dagster-webserver==1.5.9 +dagster-webserver==1.5.13 # via dagit debugpy==1.8.0 # via ipykernel @@ -154,7 +163,7 @@ dependency-injector==4.41.0 # via -r requirements/main.in deprecated==1.2.14 # via linkml-runtime -distlib==0.3.7 +distlib==0.3.8 # via virtualenv dnspython==2.4.2 # via @@ -182,7 +191,7 @@ exceptiongroup==1.2.0 # pytest executing==2.0.1 # via stack-data -fastapi==0.104.1 +fastapi==0.105.0 # via -r requirements/main.in fastjsonschema==2.19.0 # via @@ -196,9 +205,13 @@ fnc==0.5.3 # via -r requirements/main.in fqdn==1.5.1 # via jsonschema -frozendict==2.3.9 +frozendict==2.3.10 # via -r requirements/main.in -fsspec==2023.10.0 +frozenlist==1.4.1 + # via + # aiohttp + # aiosignal +fsspec==2023.12.2 # via universal-pathlib ghp-import==2.1.0 # via mkdocs @@ -219,13 +232,11 @@ graphql-relay==3.2.0 # via graphene graphviz==0.20.1 # via linkml -greenlet==3.0.1 - # via sqlalchemy -grpcio==1.59.3 +grpcio==1.60.0 # via # dagster # grpcio-health-checking -grpcio-health-checking==1.59.3 +grpcio-health-checking==1.60.0 # via dagster h11==0.14.0 # via uvicorn @@ -269,7 +280,7 @@ isodate==0.6.1 # rdflib isoduration==20.11.0 # via jsonschema -isort==5.12.0 +isort==5.13.2 # via shed jedi==0.19.1 # via ipython @@ -323,7 +334,7 @@ jsonschema==4.20.0 # linkml # linkml-runtime # nbformat -jsonschema-specifications==2023.11.1 +jsonschema-specifications==2023.11.2 # via jsonschema jupyter==1.0.0 # via -r requirements/main.in @@ -351,14 +362,14 @@ jupyter-events==0.9.0 # via jupyter-server jupyter-lsp==2.2.1 # via jupyterlab -jupyter-server==2.11.1 +jupyter-server==2.12.1 # via # jupyter-lsp # jupyterlab # jupyterlab-server # notebook # notebook-shim -jupyter-server-terminals==0.4.4 +jupyter-server-terminals==0.5.0 # via jupyter-server jupyterlab==4.0.9 # via @@ -372,19 +383,19 @@ jupyterlab-server==2.25.2 # notebook jupyterlab-widgets==3.0.9 # via ipywidgets -jupytext==1.15.2 +jupytext==1.16.0 # via mkdocs-jupyter lazy-model==0.2.0 # via beanie libcst==1.1.0 # via shed -linkml==1.6.3 +linkml==1.6.6 # via # -r requirements/main.in # nmdc-schema linkml-dataops==0.1.0 # via linkml -linkml-runtime==1.6.2 +linkml-runtime==1.6.3 # via # -r requirements/main.in # linkml @@ -426,7 +437,7 @@ mkdocs==1.5.3 # mkdocs-mermaid2-plugin mkdocs-jupyter==0.24.6 # via -r requirements/main.in -mkdocs-material==9.4.14 +mkdocs-material==9.5.2 # via # -r requirements/main.in # mkdocs-jupyter @@ -439,14 +450,16 @@ motor==3.3.2 # -r requirements/main.in # beanie multidict==6.0.4 - # via yarl + # via + # aiohttp + # yarl mypy-extensions==1.0.0 # via # black # typing-inspect nbclient==0.9.0 # via nbconvert -nbconvert==7.11.0 +nbconvert==7.12.0 # via # jupyter # jupyter-server @@ -459,7 +472,7 @@ nbformat==5.9.2 # nbconvert nest-asyncio==1.5.8 # via ipykernel -nmdc-schema==9.2.0 +nmdc-schema==9.3.1 # via -r requirements/main.in notebook==7.0.6 # via jupyter @@ -487,6 +500,7 @@ packaging==23.2 # jupyter-server # jupyterlab # jupyterlab-server + # jupytext # mkdocs # nbconvert # pyproject-api @@ -498,7 +512,7 @@ packaging==23.2 # tox paginate==0.5.6 # via mkdocs-material -pandas==2.1.3 +pandas==2.1.4 # via # -r requirements/main.in # terminusdb-client @@ -510,7 +524,7 @@ parso==0.8.3 # via jedi passlib==1.7.4 # via -r requirements/main.in -pathspec==0.11.2 +pathspec==0.12.1 # via # black # mkdocs @@ -518,7 +532,7 @@ pendulum==2.1.2 # via dagster pexpect==4.9.0 # via ipython -platformdirs==4.0.0 +platformdirs==4.1.0 # via # black # jupyter-core @@ -542,7 +556,7 @@ prefixmaps==0.2.0 # linkml-runtime prometheus-client==0.19.0 # via jupyter-server -prompt-toolkit==3.0.41 +prompt-toolkit==3.0.43 # via # ipython # jupyter-console @@ -669,7 +683,7 @@ pyyaml==6.0.1 # uvicorn pyyaml-env-tag==0.1 # via mkdocs -pyzmq==25.1.1 +pyzmq==25.1.2 # via # ipykernel # jupyter-client @@ -696,7 +710,7 @@ rdflib-shim==1.0.3 # pyshex # pyshexc # sparqlslurper -referencing==0.31.1 +referencing==0.32.0 # via # jsonschema # jsonschema-specifications @@ -742,12 +756,10 @@ rpds-py==0.13.2 rsa==4.9 # via python-jose ruamel-yaml==0.18.5 - # via - # linkml-dataops - # nmdc-schema + # via linkml-dataops ruamel-yaml-clib==0.2.8 # via ruamel-yaml -s3transfer==0.8.2 +s3transfer==0.9.0 # via boto3 semver==3.0.2 # via -r requirements/main.in @@ -890,7 +902,7 @@ typeguard==2.13.3 # via terminusdb-client types-python-dateutil==2.8.19.14 # via arrow -typing-extensions==4.8.0 +typing-extensions==4.9.0 # via # alembic # async-lru @@ -929,7 +941,7 @@ uvicorn==0.24.0.post1 # dagster-webserver uvloop==0.19.0 # via uvicorn -virtualenv==20.24.7 +virtualenv==20.25.0 # via tox watchdog==3.0.0 # via @@ -946,7 +958,7 @@ webencodings==0.5.1 # via # bleach # tinycss2 -websocket-client==1.6.4 +websocket-client==1.7.0 # via jupyter-server websockets==12.0 # via uvicorn @@ -958,8 +970,10 @@ xlrd==2.0.1 # via -r requirements/main.in xlsxwriter==3.1.9 # via -r requirements/main.in -yarl==1.9.3 - # via gql +yarl==1.9.4 + # via + # aiohttp + # gql # The following packages are considered to be unsafe in a requirements file: setuptools==69.0.2 From 8d714dfe820808b14c4d9fa268f2b59bdb17b312 Mon Sep 17 00:00:00 2001 From: Patrick Kalita Date: Fri, 15 Dec 2023 16:38:26 -0800 Subject: [PATCH 05/23] Upgrade to nmdc-schema 9.3.2 --- requirements/main.in | 2 +- requirements/main.txt | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements/main.in b/requirements/main.in index 7fdc1e67..4509db70 100644 --- a/requirements/main.in +++ b/requirements/main.in @@ -24,7 +24,7 @@ mkdocs-jupyter mkdocs-material mkdocs-mermaid2-plugin motor -nmdc-schema==9.3.1 +nmdc-schema==9.3.2 openpyxl pandas passlib[bcrypt] diff --git a/requirements/main.txt b/requirements/main.txt index 83caa909..0bec54d1 100644 --- a/requirements/main.txt +++ b/requirements/main.txt @@ -72,9 +72,9 @@ black==23.12.0 # via shed bleach==6.1.0 # via nbconvert -boto3==1.34.1 +boto3==1.34.2 # via -r requirements/main.in -botocore==1.34.1 +botocore==1.34.2 # via # boto3 # s3transfer @@ -472,7 +472,7 @@ nbformat==5.9.2 # nbconvert nest-asyncio==1.5.8 # via ipykernel -nmdc-schema==9.3.1 +nmdc-schema==9.3.2 # via -r requirements/main.in notebook==7.0.6 # via jupyter From abe2e5dfa23bb9d9069b695688cc429c1616c1e5 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 3 Jan 2024 15:33:54 -0500 Subject: [PATCH 06/23] fix: functional_annotation_agg closes #414 --- nmdc_runtime/api/endpoints/util.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/nmdc_runtime/api/endpoints/util.py b/nmdc_runtime/api/endpoints/util.py index f8279efb..4655ed54 100644 --- a/nmdc_runtime/api/endpoints/util.py +++ b/nmdc_runtime/api/endpoints/util.py @@ -110,7 +110,9 @@ def list_resources(req: ListRequest, mdb: MongoDatabase, collection_name: str): } return rv else: - if "id_1" not in mdb[collection_name].index_information(): + # the below line committed in anger. nmdc schema collections should have an 'id' field. + id_field = "_id" if collection_name == "functional_annotation_agg" else "id" + if id_field == "id" and "id_1" not in mdb[collection_name].index_information(): logging.warning( f"list_resources: no index set on 'id' for collection {collection_name}" ) @@ -119,11 +121,11 @@ def list_resources(req: ListRequest, mdb: MongoDatabase, collection_name: str): filter=filter_, projection=projection, limit=limit, - sort=[("id", 1)], + sort=[(id_field, 1)], allow_disk_use=True, ) ) - last_id = resources[-1]["id"] + last_id = resources[-1][id_field] token = generate_one_id(mdb, "page_tokens") mdb.page_tokens.insert_one( {"_id": token, "ns": collection_name, "last_id": last_id} From 97f62ff6762f7c9a7746ba2863abe42badb04576 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 3 Jan 2024 15:39:34 -0500 Subject: [PATCH 07/23] feat: make a bit more resilient --- nmdc_runtime/api/endpoints/util.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nmdc_runtime/api/endpoints/util.py b/nmdc_runtime/api/endpoints/util.py index 4655ed54..b6cda5da 100644 --- a/nmdc_runtime/api/endpoints/util.py +++ b/nmdc_runtime/api/endpoints/util.py @@ -110,12 +110,13 @@ def list_resources(req: ListRequest, mdb: MongoDatabase, collection_name: str): } return rv else: - # the below line committed in anger. nmdc schema collections should have an 'id' field. - id_field = "_id" if collection_name == "functional_annotation_agg" else "id" - if id_field == "id" and "id_1" not in mdb[collection_name].index_information(): + # the below block committed in anger. nmdc schema collections should have an 'id' field. + id_field = "id" + if "id_1" not in mdb[collection_name].index_information(): logging.warning( f"list_resources: no index set on 'id' for collection {collection_name}" ) + id_field = "_id" # expected atm for functional_annotation_agg resources = list( mdb[collection_name].find( filter=filter_, From 18fea448abcc8b28b0f5ed3a33055030055ebd7d Mon Sep 17 00:00:00 2001 From: Mark Andrew Miller Date: Wed, 10 Jan 2024 13:46:09 -0500 Subject: [PATCH 08/23] docker-compose -> docker compose --- .gitpod.yml | 2 +- Makefile | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.gitpod.yml b/.gitpod.yml index f8ebd20d..5aa5f971 100644 --- a/.gitpod.yml +++ b/.gitpod.yml @@ -9,4 +9,4 @@ tasks: - name: Start Dev on Fresh Gitpod before: cp .env.example .env init: docker compose up mongo --detach && make mongorestore-nmdc-dev - command: make up-dev && docker-compose logs -f fastapi + command: make up-dev && docker compose logs -f fastapi diff --git a/Makefile b/Makefile index 6316a2fd..faf13225 100644 --- a/Makefile +++ b/Makefile @@ -18,14 +18,14 @@ update-deps: update: update-deps init up-dev: - docker-compose up --build --force-recreate --detach --remove-orphans + docker compose up --build --force-recreate --detach --remove-orphans dev-reset-db: docker compose \ exec mongo /bin/bash -c "./app_tests/mongorestore-nmdc-testdb.sh" up-test: - docker-compose --file docker-compose.test.yml \ + docker compose --file docker-compose.test.yml \ up --build --force-recreate --detach --remove-orphans test-build: @@ -49,13 +49,13 @@ lint: --statistics --extend-exclude="./build/" --extend-ignore=F722 down-dev: - docker-compose down + docker compose down down-test: - docker-compose --file docker-compose.test.yml down + docker compose --file docker-compose.test.yml down follow-fastapi: - docker-compose logs fastapi -f + docker compose logs fastapi -f fastapi-deploy-spin: rancher kubectl rollout restart deployment/runtime-fastapi --namespace=nmdc-dev From 39d9a8a9f596fa5558fba58c661479586fd60a8f Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 12:00:24 -0500 Subject: [PATCH 09/23] style: black --- .../translation/neon_benthic_translator.py | 26 ++++--- .../site/translation/neon_soil_translator.py | 31 ++++---- nmdc_runtime/site/translation/neon_utils.py | 71 ++++++++++--------- .../test_neon_soil_data_translator.py | 15 ++-- 4 files changed, 74 insertions(+), 69 deletions(-) diff --git a/nmdc_runtime/site/translation/neon_benthic_translator.py b/nmdc_runtime/site/translation/neon_benthic_translator.py index d3f051bb..dad7e146 100644 --- a/nmdc_runtime/site/translation/neon_benthic_translator.py +++ b/nmdc_runtime/site/translation/neon_benthic_translator.py @@ -8,7 +8,15 @@ from nmdc_schema import nmdc from nmdc_runtime.site.translation.translator import Translator from nmdc_runtime.site.util import get_basename -from nmdc_runtime.site.translation.neon_utils import _get_value_or_none, _create_controlled_identified_term_value, _create_controlled_term_value, _create_geolocation_value, _create_quantity_value, _create_timestamp_value, _create_text_value +from nmdc_runtime.site.translation.neon_utils import ( + _get_value_or_none, + _create_controlled_identified_term_value, + _create_controlled_term_value, + _create_geolocation_value, + _create_quantity_value, + _create_timestamp_value, + _create_text_value, +) BENTHIC_BROAD_SCALE_MAPPINGS = { @@ -210,9 +218,7 @@ def _translate_library_preparation( :return: Object that using LibraryPreparation process model. """ processing_institution = None - laboratory_name = _get_value_or_none( - library_preparation_row, "laboratoryName" - ) + laboratory_name = _get_value_or_none(library_preparation_row, "laboratoryName") if laboratory_name is not None: if re.search("Battelle", laboratory_name, re.IGNORECASE): processing_institution = "Battelle" @@ -263,9 +269,7 @@ def _translate_omics_processing( has_input=processed_sample_id, has_output=raw_data_file_data, processing_institution=processing_institution, - ncbi_project_name=_get_value_or_none( - omics_processing_row, "ncbiProjectID" - ), + ncbi_project_name=_get_value_or_none(omics_processing_row, "ncbiProjectID"), omics_type=_create_controlled_term_value( omics_processing_row["investigation_type"].values[0] ), @@ -434,9 +438,7 @@ def get_database(self): ) for neon_id, nmdc_id in neon_to_nmdc_biosample_ids.items(): - biosample_row = benthic_samples[ - benthic_samples["sampleID"] == neon_id - ] + biosample_row = benthic_samples[benthic_samples["sampleID"] == neon_id] database.biosample_set.append( self._translate_biosample(neon_id, nmdc_id, biosample_row) @@ -458,7 +460,9 @@ def get_database(self): ) ) - genomics_sample_id = _get_value_or_none(extraction_row, "genomicsSampleID") + genomics_sample_id = _get_value_or_none( + extraction_row, "genomicsSampleID" + ) database.processed_sample_set.append( self._translate_processed_sample( diff --git a/nmdc_runtime/site/translation/neon_soil_translator.py b/nmdc_runtime/site/translation/neon_soil_translator.py index 68a53834..4569d5a1 100644 --- a/nmdc_runtime/site/translation/neon_soil_translator.py +++ b/nmdc_runtime/site/translation/neon_soil_translator.py @@ -7,7 +7,16 @@ from nmdc_schema import nmdc from nmdc_runtime.site.translation.translator import Translator from nmdc_runtime.site.util import get_basename -from nmdc_runtime.site.translation.neon_utils import _get_value_or_none, _create_controlled_identified_term_value, _create_controlled_term_value, _create_geolocation_value, _create_quantity_value, _create_timestamp_value, _create_text_value, _create_double_value +from nmdc_runtime.site.translation.neon_utils import ( + _get_value_or_none, + _create_controlled_identified_term_value, + _create_controlled_term_value, + _create_geolocation_value, + _create_quantity_value, + _create_timestamp_value, + _create_text_value, + _create_double_value, +) class NeonSoilDataTranslator(Translator): @@ -124,9 +133,7 @@ def _translate_biosample( collection_date=_create_timestamp_value( biosample_row["collectDate"].values[0] ), - temp=_create_quantity_value( - biosample_row["soilTemp"].values[0], "Celsius" - ), + temp=_create_quantity_value(biosample_row["soilTemp"].values[0], "Celsius"), depth=nmdc.QuantityValue( has_minimum_numeric_value=_get_value_or_none( biosample_row, "sampleTopDepth" @@ -136,13 +143,9 @@ def _translate_biosample( ), has_unit="m", ), - samp_collec_device=_get_value_or_none( - biosample_row, "soilSamplingDevice" - ), + samp_collec_device=_get_value_or_none(biosample_row, "soilSamplingDevice"), soil_horizon=_get_value_or_none(biosample_row, "horizon"), - analysis_type=_get_value_or_none( - biosample_row, "sequenceAnalysisType" - ), + analysis_type=_get_value_or_none(biosample_row, "sequenceAnalysisType"), env_package=_create_text_value(biosample_row["sampleType"].values[0]), nitro=_create_quantity_value( biosample_row["nitrogenPercent"].values[0], "percent" @@ -301,9 +304,7 @@ def _translate_library_preparation( :return: Object that using LibraryPreparation process model. """ processing_institution = None - laboratory_name = _get_value_or_none( - library_preparation_row, "laboratoryName" - ) + laboratory_name = _get_value_or_none(library_preparation_row, "laboratoryName") if laboratory_name is not None: if re.search("Battelle", laboratory_name, re.IGNORECASE): processing_institution = "Battelle" @@ -354,9 +355,7 @@ def _translate_omics_processing( has_input=processed_sample_id, has_output=raw_data_file_data, processing_institution=processing_institution, - ncbi_project_name=_get_value_or_none( - omics_processing_row, "ncbiProjectID" - ), + ncbi_project_name=_get_value_or_none(omics_processing_row, "ncbiProjectID"), omics_type=_create_controlled_term_value( omics_processing_row["investigation_type"].values[0] ), diff --git a/nmdc_runtime/site/translation/neon_utils.py b/nmdc_runtime/site/translation/neon_utils.py index 97d74de3..290e0613 100644 --- a/nmdc_runtime/site/translation/neon_utils.py +++ b/nmdc_runtime/site/translation/neon_utils.py @@ -5,35 +5,34 @@ from nmdc_schema import nmdc -def _get_value_or_none( - data: pd.DataFrame, column_name: str - ) -> Union[str, float, None]: - """ - Get the value from the specified column in the data DataFrame. - If the column value is NaN, return None. However, there are handlers - for a select set of columns - horizon, qaqcStatus, sampleTopDepth, - and sampleBottomDepth. - - :param data: DataFrame to read the column value from. - :return: Either a string, float or None depending on the column/column values. - """ - if ( - column_name in data - and not data[column_name].isna().any() - and not data[column_name].empty - ): - if column_name == "horizon": - return f"{data[column_name].values[0]} horizon" - elif column_name == "qaqcStatus": - return data[column_name].values[0].lower() - elif column_name == "sampleTopDepth": - return float(data[column_name].values[0]) / 100 - elif column_name == "sampleBottomDepth": - return float(data[column_name].values[0]) / 100 - else: - return data[column_name].values[0] +def _get_value_or_none(data: pd.DataFrame, column_name: str) -> Union[str, float, None]: + """ + Get the value from the specified column in the data DataFrame. + If the column value is NaN, return None. However, there are handlers + for a select set of columns - horizon, qaqcStatus, sampleTopDepth, + and sampleBottomDepth. + + :param data: DataFrame to read the column value from. + :return: Either a string, float or None depending on the column/column values. + """ + if ( + column_name in data + and not data[column_name].isna().any() + and not data[column_name].empty + ): + if column_name == "horizon": + return f"{data[column_name].values[0]} horizon" + elif column_name == "qaqcStatus": + return data[column_name].values[0].lower() + elif column_name == "sampleTopDepth": + return float(data[column_name].values[0]) / 100 + elif column_name == "sampleBottomDepth": + return float(data[column_name].values[0]) / 100 + else: + return data[column_name].values[0] + + return None - return None def _create_controlled_identified_term_value( id: str = None, name: str = None @@ -47,13 +46,10 @@ def _create_controlled_identified_term_value( """ if id is None or name is None: return None - return nmdc.ControlledIdentifiedTermValue( - term=nmdc.OntologyClass(id=id, name=name) - ) + return nmdc.ControlledIdentifiedTermValue(term=nmdc.OntologyClass(id=id, name=name)) -def _create_controlled_term_value( - name: str = None -) -> nmdc.ControlledTermValue: + +def _create_controlled_term_value(name: str = None) -> nmdc.ControlledTermValue: """ Create a ControlledIdentifiedTermValue object with the specified id and name. @@ -66,6 +62,7 @@ def _create_controlled_term_value( return None return nmdc.ControlledTermValue(has_raw_value=name) + def _create_timestamp_value(value: str = None) -> nmdc.TimestampValue: """ Create a TimestampValue object with the specified value. @@ -78,6 +75,7 @@ def _create_timestamp_value(value: str = None) -> nmdc.TimestampValue: return None return nmdc.TimestampValue(has_raw_value=value) + def _create_quantity_value( numeric_value: Union[str, int, float] = None, unit: str = None ) -> nmdc.QuantityValue: @@ -94,6 +92,7 @@ def _create_quantity_value( return None return nmdc.QuantityValue(has_numeric_value=float(numeric_value), has_unit=unit) + def _create_text_value(value: str = None) -> nmdc.TextValue: """ Create a TextValue object with the specified value. @@ -105,6 +104,7 @@ def _create_text_value(value: str = None) -> nmdc.TextValue: return None return nmdc.TextValue(has_raw_value=value) + def _create_double_value(value: str = None) -> nmdc.Double: """ Create a Double object with the specified value. @@ -117,6 +117,7 @@ def _create_double_value(value: str = None) -> nmdc.Double: return None return nmdc.Double(value) + def _create_geolocation_value( latitude: str = None, longitude: str = None ) -> nmdc.GeolocationValue: @@ -142,4 +143,4 @@ def _create_geolocation_value( return nmdc.GeolocationValue( latitude=nmdc.DecimalDegree(latitude), longitude=nmdc.DecimalDegree(longitude), - ) \ No newline at end of file + ) diff --git a/tests/test_data/test_neon_soil_data_translator.py b/tests/test_data/test_neon_soil_data_translator.py index 391833ae..e9da029b 100644 --- a/tests/test_data/test_neon_soil_data_translator.py +++ b/tests/test_data/test_neon_soil_data_translator.py @@ -2,7 +2,12 @@ import string import pytest from nmdc_runtime.site.translation.neon_soil_translator import NeonSoilDataTranslator -from nmdc_runtime.site.translation.neon_utils import (_create_controlled_identified_term_value, _create_controlled_term_value, _create_timestamp_value, _get_value_or_none) +from nmdc_runtime.site.translation.neon_utils import ( + _create_controlled_identified_term_value, + _create_controlled_term_value, + _create_timestamp_value, + _get_value_or_none, +) import pandas as pd import requests @@ -819,15 +824,11 @@ def test_get_value_or_none(self): # specific handler for depth slot expected_minimum_depth = 0.0 - actual_minimum_depth = _get_value_or_none( - test_biosample, "sampleTopDepth" - ) + actual_minimum_depth = _get_value_or_none(test_biosample, "sampleTopDepth") assert expected_minimum_depth == actual_minimum_depth expected_maximum_depth = 0.295 - actual_maximum_depth = _get_value_or_none( - test_biosample, "sampleBottomDepth" - ) + actual_maximum_depth = _get_value_or_none(test_biosample, "sampleBottomDepth") assert expected_maximum_depth == actual_maximum_depth expected_sample_id = "BLAN_005-M-8-0-20200713" From 1abf073718400b3d6e1a6ff75aeb5346f48d68eb Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 12:19:37 -0500 Subject: [PATCH 10/23] new GH action to lint and reformat --- .github/workflows/lint.yml | 30 +++++++++++++++++++++++++ Makefile | 3 +++ nmdc_runtime/containers.py | 1 - nmdc_runtime/lib/nmdc_etl_class.py | 1 - nmdc_runtime/lib/transform_nmdc_data.py | 1 - 5 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/lint.yml diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 00000000..02b070c1 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,30 @@ +name: Lint-check + Style-normalize Python files + +on: + push: + branches: [ main ] + paths: + - '.github/workflows/lint.yml' + - '**.py' + pull_request: + paths: + - '.github/workflows/lint.yml' + - '**.py' + + +jobs: + build: + name: lint-check and style-normalize Python files + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + - name: Set up Python 3.10 + uses: actions/setup-python@v4 + with: + python-version: '3.10' + - name: Lint with flake8 and Reformat with black + run: | + make init + make lint + make black diff --git a/Makefile b/Makefile index faf13225..fdb22114 100644 --- a/Makefile +++ b/Makefile @@ -41,6 +41,9 @@ test-run: test: test-build test-run +black: + black nmdc_runtime + lint: # Python syntax errors or undefined names flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics --extend-ignore=F722 diff --git a/nmdc_runtime/containers.py b/nmdc_runtime/containers.py index 977bbff7..f3283730 100644 --- a/nmdc_runtime/containers.py +++ b/nmdc_runtime/containers.py @@ -9,7 +9,6 @@ class Container(containers.DeclarativeContainer): - user_queries = providers.Singleton(UserQueries) user_service = providers.Factory(UserService, user_queries=user_queries) diff --git a/nmdc_runtime/lib/nmdc_etl_class.py b/nmdc_runtime/lib/nmdc_etl_class.py index 0f4ffb96..f9ca0a81 100644 --- a/nmdc_runtime/lib/nmdc_etl_class.py +++ b/nmdc_runtime/lib/nmdc_etl_class.py @@ -196,7 +196,6 @@ def transform_dataframe( print_df=False, print_dict=False, ) -> list: - ## used for testing if test_rows != 0: nmdc_df = nmdc_df.head(test_rows) diff --git a/nmdc_runtime/lib/transform_nmdc_data.py b/nmdc_runtime/lib/transform_nmdc_data.py index 1ab2f422..afbcbe32 100644 --- a/nmdc_runtime/lib/transform_nmdc_data.py +++ b/nmdc_runtime/lib/transform_nmdc_data.py @@ -995,7 +995,6 @@ def make_quantity_value(nmdc_objs: list, tx_attributes: list, **kwargs) -> list: for attribute in tx_attributes: for obj in nmdc_objs: if has_raw_value(obj, attribute): - val = getattr(obj, attribute) ## split raw value after first space From 9c8ee27843575748394299da5f75572c0bb07499 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 12:31:30 -0500 Subject: [PATCH 11/23] commit and push reformatting closes #438 --- .github/workflows/lint.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 02b070c1..d1cec8e8 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -28,3 +28,7 @@ jobs: make init make lint make black + - name: commit and push if reformatted + run: | + if git status --short | grep -q '\.py$'; then git add *.py && git commit -m "style: reformat" && git push; fi + From d38128b0ae7f06e4cef28b557f1f71c7339c3746 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 12:37:53 -0500 Subject: [PATCH 12/23] fix --- nmdc_runtime/api/models/object.py | 2 +- nmdc_runtime/site/translation/gold_translator.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nmdc_runtime/api/models/object.py b/nmdc_runtime/api/models/object.py index 17df772c..1fd5ced4 100644 --- a/nmdc_runtime/api/models/object.py +++ b/nmdc_runtime/api/models/object.py @@ -165,7 +165,7 @@ class DrsObjectOutBase(DrsObjectBase): version: Optional[str] = None @field_serializer("self_uri") - def serialize_url(self, slf_uri: AnyUrl, _info): + def serialize_url(self, self_uri: AnyUrl, _info): return str(self_uri) diff --git a/nmdc_runtime/site/translation/gold_translator.py b/nmdc_runtime/site/translation/gold_translator.py index 265b8b9e..42d3fe6e 100644 --- a/nmdc_runtime/site/translation/gold_translator.py +++ b/nmdc_runtime/site/translation/gold_translator.py @@ -212,7 +212,7 @@ def _get_quantity_value( return None elif minimum_numeric_value is not None and maximum_numeric_value is None: return nmdc.QuantityValue( - has_raw_value=field_value, + has_raw_value=minimum_numeric_value, has_numeric_value=nmdc.Double(minimum_numeric_value), has_unit=unit, ) From edef778db9475cec780a1d25358a3c4f8266761f Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 12:50:29 -0500 Subject: [PATCH 13/23] quicken lint GH action --- .github/workflows/lint.yml | 2 +- Makefile | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index d1cec8e8..9f4781f9 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -25,7 +25,7 @@ jobs: python-version: '3.10' - name: Lint with flake8 and Reformat with black run: | - make init + make init-lint-and-black make lint make black - name: commit and push if reformatted diff --git a/Makefile b/Makefile index fdb22114..87c02752 100644 --- a/Makefile +++ b/Makefile @@ -51,6 +51,13 @@ lint: flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 \ --statistics --extend-exclude="./build/" --extend-ignore=F722 +PIP_PINNED_FLAKE8 := $(shell grep 'flake8==' requirements/dev.txt) +PIP_PINNED_BLACK := $(shell grep 'black==' requirements/dev.txt) + +init-lint-and-black: + pip install $(PIP_PINNED_FLAKE8) + pip install $(PIP_PINNED_BLACK) + down-dev: docker compose down From 5916d58c981b5758a344342188e7fdc50aa17075 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 12:53:16 -0500 Subject: [PATCH 14/23] test: for #439 --- nmdc_runtime/api/main.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/nmdc_runtime/api/main.py b/nmdc_runtime/api/main.py index e8fe4015..dde09376 100644 --- a/nmdc_runtime/api/main.py +++ b/nmdc_runtime/api/main.py @@ -343,8 +343,6 @@ async def get_versions(): "fastapi": fastapi.__version__, "nmdc-schema": version("nmdc_schema"), } - - app = FastAPI( title="NMDC Runtime API", version=get_version(), From 2d0a604109309b076ae8a6068cafe10b9f424af5 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 12:55:48 -0500 Subject: [PATCH 15/23] fix: author-ize --- .github/workflows/lint.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 9f4781f9..3de52085 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -30,5 +30,7 @@ jobs: make black - name: commit and push if reformatted run: | + git config user.name github-actions + git config user.email github-actions@github.com if git status --short | grep -q '\.py$'; then git add *.py && git commit -m "style: reformat" && git push; fi From 72d7cf7982cc98aeae845c8ed7ecad3f33fc92ce Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 12:59:21 -0500 Subject: [PATCH 16/23] fix: quote --- .github/workflows/lint.yml | 2 +- nmdc_runtime/api/main.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 3de52085..d140dee7 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -32,5 +32,5 @@ jobs: run: | git config user.name github-actions git config user.email github-actions@github.com - if git status --short | grep -q '\.py$'; then git add *.py && git commit -m "style: reformat" && git push; fi + if git status --short | grep -q '\.py$'; then git add '*.py' && git commit -m "style: reformat" && git push; fi diff --git a/nmdc_runtime/api/main.py b/nmdc_runtime/api/main.py index dde09376..feb936e9 100644 --- a/nmdc_runtime/api/main.py +++ b/nmdc_runtime/api/main.py @@ -334,8 +334,6 @@ async def root(): BASE_URL_EXTERNAL + "/docs", status_code=status.HTTP_303_SEE_OTHER, ) - - @api_router.get("/version") async def get_versions(): return { From f893c55432a094bb261741573b7795bcfa53fb87 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 13:01:46 -0500 Subject: [PATCH 17/23] fix: autosetup remote --- .github/workflows/lint.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index d140dee7..c48b5f56 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -32,5 +32,6 @@ jobs: run: | git config user.name github-actions git config user.email github-actions@github.com + git config push.autoSetupRemote true if git status --short | grep -q '\.py$'; then git add '*.py' && git commit -m "style: reformat" && git push; fi From 9c5c1f11cf4bee128f46d0ef23129cc5d64c386d Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 13:06:26 -0500 Subject: [PATCH 18/23] fix: ensure HEAD ref for git push --- .github/workflows/lint.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c48b5f56..12f1eec2 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,11 +1,6 @@ name: Lint-check + Style-normalize Python files on: - push: - branches: [ main ] - paths: - - '.github/workflows/lint.yml' - - '**.py' pull_request: paths: - '.github/workflows/lint.yml' @@ -23,6 +18,7 @@ jobs: uses: actions/setup-python@v4 with: python-version: '3.10' + ref: ${{ github.event.pull_request.head.ref }} - name: Lint with flake8 and Reformat with black run: | make init-lint-and-black @@ -32,6 +28,5 @@ jobs: run: | git config user.name github-actions git config user.email github-actions@github.com - git config push.autoSetupRemote true if git status --short | grep -q '\.py$'; then git add '*.py' && git commit -m "style: reformat" && git push; fi From 9585ba14c4c1c25f960c0c3692ac38461ae8fd29 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 13:14:53 -0500 Subject: [PATCH 19/23] try .sha --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 12f1eec2..b22f4bab 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -18,7 +18,7 @@ jobs: uses: actions/setup-python@v4 with: python-version: '3.10' - ref: ${{ github.event.pull_request.head.ref }} + ref: ${{ github.event.pull_request.head.sha }} - name: Lint with flake8 and Reformat with black run: | make init-lint-and-black From d7302eb99d262f28893c62bdad1ec14666b878f1 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Wed, 17 Jan 2024 13:17:52 -0500 Subject: [PATCH 20/23] fix: arg sent to wrong step --- .github/workflows/lint.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index b22f4bab..1bc016af 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -14,11 +14,12 @@ jobs: steps: - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.ref }} - name: Set up Python 3.10 uses: actions/setup-python@v4 with: python-version: '3.10' - ref: ${{ github.event.pull_request.head.sha }} - name: Lint with flake8 and Reformat with black run: | make init-lint-and-black From 6c49f68cdcd6a264f5004353fa93031a79f267a0 Mon Sep 17 00:00:00 2001 From: github-actions Date: Wed, 17 Jan 2024 18:18:21 +0000 Subject: [PATCH 21/23] style: reformat --- nmdc_runtime/api/main.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nmdc_runtime/api/main.py b/nmdc_runtime/api/main.py index feb936e9..e8fe4015 100644 --- a/nmdc_runtime/api/main.py +++ b/nmdc_runtime/api/main.py @@ -334,6 +334,8 @@ async def root(): BASE_URL_EXTERNAL + "/docs", status_code=status.HTTP_303_SEE_OTHER, ) + + @api_router.get("/version") async def get_versions(): return { @@ -341,6 +343,8 @@ async def get_versions(): "fastapi": fastapi.__version__, "nmdc-schema": version("nmdc_schema"), } + + app = FastAPI( title="NMDC Runtime API", version=get_version(), From d989ebd58c9193c916b79ca23f0a755eeb2e3916 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Mon, 22 Jan 2024 13:30:32 -0500 Subject: [PATCH 22/23] cookie auth (#441) * feat: login-with-orcid link at top for #423` * new GH action to lint and reformat * commit and push reformatting closes #438 * fix * quicken lint GH action * test: for #439 * fix: author-ize * fix: quote * fix: autosetup remote * fix: ensure HEAD ref for git push * try .sha * fix: arg sent to wrong step * style: reformat * inprogress: do not merge * inprogress: do not merge * inprogress: do not merge: add todo * [do not merge] login w/o logout * remove old orcid endpoints * remove auth-action tags * remove commented out orcid_cookie_test * clean: abandon auth-action hack for now --------- Co-authored-by: github-actions Co-authored-by: Jing Cao --- .env.example | 3 +- nmdc_runtime/api/core/auth.py | 31 ++++++-- nmdc_runtime/api/endpoints/users.py | 60 ++++++--------- nmdc_runtime/api/main.py | 77 ++++++++++++++++++-- nmdc_runtime/api/models/user.py | 2 + nmdc_runtime/static/ORCIDiD_icon128x128.png | Bin 0 -> 3410 bytes 6 files changed, 126 insertions(+), 47 deletions(-) create mode 100644 nmdc_runtime/static/ORCIDiD_icon128x128.png diff --git a/.env.example b/.env.example index b9505de9..03baec27 100644 --- a/.env.example +++ b/.env.example @@ -37,4 +37,5 @@ NEON_API_TOKEN=y NEON_API_BASE_URL=https://data.neonscience.org/api/v0 NERSC_USERNAME=replaceme -ORCID_CLIENT_ID=replaceme \ No newline at end of file +ORCID_CLIENT_ID=replaceme +ORCID_CLIENT_SECRET=replaceme \ No newline at end of file diff --git a/nmdc_runtime/api/core/auth.py b/nmdc_runtime/api/core/auth.py index 5e4d7c1c..85c5d5a5 100644 --- a/nmdc_runtime/api/core/auth.py +++ b/nmdc_runtime/api/core/auth.py @@ -6,17 +6,25 @@ from fastapi.exceptions import HTTPException from fastapi.openapi.models import OAuthFlows as OAuthFlowsModel from fastapi.param_functions import Form -from fastapi.security import OAuth2, HTTPBasic, HTTPBasicCredentials +from fastapi.security import ( + OAuth2, + HTTPBasic, + HTTPBasicCredentials, + HTTPBearer, + HTTPAuthorizationCredentials, +) from fastapi.security.utils import get_authorization_scheme_param from jose import JWTError, jwt from passlib.context import CryptContext from pydantic import BaseModel +from starlette import status from starlette.requests import Request from starlette.status import HTTP_400_BAD_REQUEST, HTTP_401_UNAUTHORIZED SECRET_KEY = os.getenv("JWT_SECRET_KEY") ALGORITHM = "HS256" ORCID_CLIENT_ID = os.getenv("ORCID_CLIENT_ID") +ORCID_CLIENT_SECRET = os.getenv("ORCID_CLIENT_SECRET") # https://orcid.org/.well-known/openid-configuration # XXX do we want to live-load this? @@ -129,15 +137,24 @@ async def __call__(self, request: Request) -> Optional[str]: tokenUrl="token", auto_error=False ) +bearer_scheme = HTTPBearer(scheme_name="bearerAuth", auto_error=False) + async def basic_credentials(req: Request): return await HTTPBasic(auto_error=False)(req) +async def bearer_credentials(req: Request): + return await HTTPBearer(scheme_name="bearerAuth", auto_error=False)(req) + + class OAuth2PasswordOrClientCredentialsRequestForm: def __init__( self, basic_creds: Optional[HTTPBasicCredentials] = Depends(basic_credentials), + bearer_creds: Optional[HTTPAuthorizationCredentials] = Depends( + bearer_credentials + ), grant_type: str = Form(None, regex="^password$|^client_credentials$"), username: Optional[str] = Form(None), password: Optional[str] = Form(None), @@ -145,14 +162,18 @@ def __init__( client_id: Optional[str] = Form(None), client_secret: Optional[str] = Form(None), ): - if grant_type == "password" and (username is None or password is None): + if bearer_creds: + self.grant_type = "client_credentials" + self.username, self.password = None, None + self.scopes = scope.split() + self.client_id = bearer_creds.credentials + self.client_secret = None + elif grant_type == "password" and (username is None or password is None): raise HTTPException( status_code=HTTP_400_BAD_REQUEST, detail="grant_type password requires username and password", ) - if grant_type == "client_credentials" and ( - client_id is None or client_secret is None - ): + elif grant_type == "client_credentials" and (client_id is None): if basic_creds: client_id = basic_creds.username client_secret = basic_creds.password diff --git a/nmdc_runtime/api/endpoints/users.py b/nmdc_runtime/api/endpoints/users.py index 4f79e752..c174092c 100644 --- a/nmdc_runtime/api/endpoints/users.py +++ b/nmdc_runtime/api/endpoints/users.py @@ -2,7 +2,9 @@ from datetime import timedelta import pymongo.database +import requests from fastapi import Depends, APIRouter, HTTPException, status +from fastapi.openapi.docs import get_swagger_ui_html from jose import jws, JWTError from starlette.requests import Request from starlette.responses import HTMLResponse, RedirectResponse @@ -16,6 +18,7 @@ ORCID_JWK, ORCID_JWS_VERITY_ALGORITHM, credentials_exception, + ORCID_CLIENT_SECRET, ) from nmdc_runtime.api.core.auth import get_password_hash from nmdc_runtime.api.core.util import generate_secret @@ -32,43 +35,28 @@ router = APIRouter() -@router.get("/orcid_authorize") -async def orcid_authorize(): - """NOTE: You want to load /orcid_authorize directly in your web browser to initiate the login redirect flow.""" - return RedirectResponse( - f"https://orcid.org/oauth/authorize?client_id={ORCID_CLIENT_ID}" - "&response_type=token&scope=openid&" - f"redirect_uri={BASE_URL_EXTERNAL}/orcid_token" - ) - - -@router.get("/orcid_token") -async def redirect_uri_for_orcid_token(req: Request): - """ - Returns a web page that will display a user's orcid jwt token for copy/paste. - - This route is loaded by orcid.org after a successful orcid user login. - """ - return HTMLResponse( - """ - - - - -
- - - """ +@router.get("/orcid_code", response_class=RedirectResponse) +async def receive_orcid_code(request: Request, code: str, state: str | None = None): + rv = requests.post( + "https://orcid.org/oauth/token", + data=( + f"client_id={ORCID_CLIENT_ID}&client_secret={ORCID_CLIENT_SECRET}&" + f"grant_type=authorization_code&code={code}&redirect_uri={BASE_URL_EXTERNAL}/orcid_code" + ), + headers={ + "Content-type": "application/x-www-form-urlencoded", + "Accept": "application/json", + }, ) + token_response = rv.json() + response = RedirectResponse(state or request.url_for("custom_swagger_ui_html")) + for key in ["user_orcid", "user_name", "user_id_token"]: + response.set_cookie( + key=key, + value=token_response[key.replace("user_", "")], + max_age=2592000, + ) + return response @router.post("/token", response_model=Token) diff --git a/nmdc_runtime/api/main.py b/nmdc_runtime/api/main.py index e8fe4015..e2107595 100644 --- a/nmdc_runtime/api/main.py +++ b/nmdc_runtime/api/main.py @@ -1,19 +1,27 @@ import os +import re from contextlib import asynccontextmanager from importlib import import_module from importlib.metadata import version +from typing import Annotated import fastapi +import requests import uvicorn -from fastapi import APIRouter, FastAPI +from fastapi import APIRouter, FastAPI, Cookie from fastapi.middleware.cors import CORSMiddleware +from fastapi.openapi.docs import get_swagger_ui_html +from fastapi.staticfiles import StaticFiles from setuptools_scm import get_version from starlette import status -from starlette.responses import RedirectResponse +from starlette.responses import RedirectResponse, HTMLResponse from nmdc_runtime.api.analytics import Analytics -from nmdc_runtime.util import all_docs_have_unique_id, ensure_unique_id_indexes -from nmdc_runtime.api.core.auth import get_password_hash +from nmdc_runtime.util import ( + ensure_unique_id_indexes, + REPO_ROOT_DIR, +) +from nmdc_runtime.api.core.auth import get_password_hash, ORCID_CLIENT_ID from nmdc_runtime.api.db.mongo import ( get_mongo_db, ) @@ -356,10 +364,15 @@ async def get_versions(): "\n\n" "Dependency versions:\n\n" f'nmdc-schema={version("nmdc_schema")}\n\n' - "Documentation" + "Documentation\n\n" + ' ' + f'Login with ORCiD' ), openapi_tags=tags_metadata, lifespan=lifespan, + docs_url=None, ) app.include_router(api_router) @@ -372,6 +385,60 @@ async def get_versions(): allow_headers=["*"], ) app.add_middleware(Analytics) +app.mount( + "/static", + StaticFiles(directory=REPO_ROOT_DIR.joinpath("nmdc_runtime/static/")), + name="static", +) + + +@app.get("/docs", include_in_schema=False) +def custom_swagger_ui_html( + user_id_token: Annotated[str | None, Cookie()] = None, +): + access_token = None + if user_id_token: + # get bearer token + rv = requests.post( + url=f"{BASE_URL_EXTERNAL}/token", + data={ + "client_id": user_id_token, + "client_secret": "", + "grant_type": "client_credentials", + }, + headers={ + "Content-type": "application/x-www-form-urlencoded", + "Accept": "application/json", + }, + ) + if rv.status_code != 200: + rv.reason = rv.text + rv.raise_for_status() + access_token = rv.json()["access_token"] + + swagger_ui_parameters = {"withCredentials": True} + if access_token is not None: + swagger_ui_parameters.update( + { + "onComplete": f"""() => {{ ui.preauthorizeApiKey(bearerAuth, {access_token}) }}""", + } + ) + response = get_swagger_ui_html( + openapi_url=app.openapi_url, + title=app.title, + oauth2_redirect_url=app.swagger_ui_oauth2_redirect_url, + swagger_js_url="https://cdn.jsdelivr.net/npm/swagger-ui-dist@5.9.0/swagger-ui-bundle.js", + swagger_css_url="https://cdn.jsdelivr.net/npm/swagger-ui-dist@5.9.0/swagger-ui.css", + swagger_ui_parameters=swagger_ui_parameters, + ) + content = ( + response.body.decode() + .replace('"', "") + .replace('"', "") + .replace("", '"') + .replace("", '"') + ) + return HTMLResponse(content=content) if __name__ == "__main__": diff --git a/nmdc_runtime/api/models/user.py b/nmdc_runtime/api/models/user.py index 0a96e2eb..dd803f59 100644 --- a/nmdc_runtime/api/models/user.py +++ b/nmdc_runtime/api/models/user.py @@ -12,6 +12,7 @@ oauth2_scheme, credentials_exception, TokenData, + bearer_scheme, ) from nmdc_runtime.api.db.mongo import get_mongo_db @@ -49,6 +50,7 @@ def authenticate_user(mdb, username: str, password: str): async def get_current_user( token: str = Depends(oauth2_scheme), + bearer_credentials: str = Depends(bearer_scheme), mdb: pymongo.database.Database = Depends(get_mongo_db), ) -> UserInDB: if mdb.invalidated_tokens.find_one({"_id": token}): diff --git a/nmdc_runtime/static/ORCIDiD_icon128x128.png b/nmdc_runtime/static/ORCIDiD_icon128x128.png new file mode 100644 index 0000000000000000000000000000000000000000..d73bdcbe895a4eca294b91527b051ee4134fbb05 GIT binary patch literal 3410 zcmbVPcT^ME9-f3iLWEGH3lh2tqyQlyN=qh@(3=er2q8d(keCFd386@_pacX_5yXO^ zAS$9F&9VyWJ`t6oh^*2SDWb@-3cld#zW2`AvwyrfXXf5J^L^j1+~4n>Gr8WL>lKkY zNB{s787`o&bQCY$2s!EVwgdBlbkG*jHjDgt;i8>P0UK}*FqbfxcrIVc27q;r@q8vLiY-EivLiS#_J&h8S`E>hFnhyI z1P`1CpURHpxFibLeuB`TgeA)kT<8RSI{{%i8>&q7MVg)SeeuNwSBujJm&xV$OQg6s!0*-W3n9(4Q z70YGEh!~)~q4WzTj1xx25lJ{ao@j$3TH$O-cxm`HR6A<|-kNR;I+F;*pBjJ1wFRjJ zYbl&gClK*?8lFUSCebJ)k}aJ`p@KNOpIk0Yim$v86XhF7s6UPpt3wT`g_xzGMe`B9cC({2X zZUUVK67BF*TRaVC1A>M>xMBZK&ahHvuuH}9U&ZqCm9zqvrhk>bbn#d7uw$ehBapVn zrlaCc0FX~-fX@E$&);wJid5UAm1=!AhalNsu|a3dRN1nZB5@mczT8k;QExioJgu6s zUa9X`;%cp%)FfBvgpVW;ro;_(J6&vh&OQ>$C`Vp_SX1`c(q4uHRMmIRc?`6Z`ugU= z=N69kO~G%Q9*GVgol9z+`ZDm!RDusY?s%*c%U2(J=6WIKbMW+$<;h2_nl<`*rW@%O zRGXWgG!vUG?6GDtnVMRCyW|S?6fj8$jRuB(6r%cim*Gwlzy0LVWSbeSVWn6XbO zWm&x3C|>JcNmIxhz5_osSNW6~;jG|~xV#G3(PfZTcA^L8HgXBzr^_dL_}~kDfDd;x zwLjQNJPJ#c?{ZbrGwu)@lyz?pQKR)A9DLTJszY}lLF5^{PMc=zy-!+kzujQ@hKwlj zSD!yF?Hg)^#cMox-xu}-viV#Y!&&c@cg)~+L;d@Wvc|!^iWJh8%cJHGR|_6*08@woEjFUd2RloV@DL(7y9let9)a!cCIgZS z>#&ClCSbMTOoqFD$0l=Vm3q3-#bGSkG2pS0k?u7~dl}WCp6W{Co8%v8DC;`bR+aHp z_wvFz0aI<|GB{y&xy>_{^O|{Lm^#I~WAwb)%M;KjxkrL5YA5Z_rnYLyxE zteo-;NhllO=>8IOw&lg}y;u2WkBX`nA5LT!w+BCZ3{lO5PD;qx)Ryi6hqKT;QOaa- zvQPiQM`THGiBmwqO`~H$lpwE={K)t3c6%PzFZ$PfNy-cJZ7aTgo+5AAawE;M)pkfb z`(&=3_{t(XUB;dRuV2g~Y4YuA^gABWUpe9PWkSYzx|%scv9(LeNZ_EmL>ag5k%dr8 z0|_}FcMQKH8`t~fl4VNW=)lRSThD;0=CHwzS8gf8zA2x#24q#iE8HHNA4S`MU7goM#qFRm&jl>kvNj1^qO-}`V4n|bwf*%cFo zXbs@9T;RhhZv=00?Rd5vAD%IW%7$o81gA!duOAph+GeKl)%ln5bHtnxhV^Vsz4}$I z<6vs(Nw8a|=eIfAne@`dRh1B+=Zz0UO@{$=R@OB-1YfNgJ&O){*+&h^Pu*;<^Faj_T8%>|NE=%f~FPUsfZ>a<7zWNOzly$&v0(73| zXV$p|^#?SBV1Qa!bEoEu=57$!)}cdnMqPu$+l^?SbAUB6`=@2a^5v!vFYFTvp{q;? zjZP3)=booyZYbLJl>IEZ8jPtp5walHcvR8xUl-n2Gzd1j%C>_KLs9kkO& zz8p|6%UC6r%Tj#tvK-h)ZlkIJ6#?p;iA5z^~Sk?&kEO8Y5=C`dsL!zez`pXFBVPM zZB8@*be4GodpfeY%y{u7ubw;u$Y_ke3gSBO;6mKWP%Xz-t!Kv@g7|s^h9hfk#-?`- zP9YnY`^v3WA+J~ zwnG#`_gv@Af=?p9$l0r(wzLK>UTi7zKmmv8ket4?7SDShH$5tQ3XxfRC3+Dt^zE9+ zb8f;byOmQNqNu1MH!%Lkt+7||7d`zm#J<#eB>JIF_{II_WQ~*0v@5+xHGv~6pjY>A z0P2!g&Dw;2*)Wk>AcNBc;L`^&35YnkG`-sjk|~xwwmhqQk|*`0!$HlIT6%225I<1prt{&94Xt~M{p9KJPsK8b%O zKdYpPaAomY0kwuq*4X~GW^3Ht2c(;C1DocmZ!k`eo~o-}e{aT9NuQ?aVvq5jS^s9S z;3_6W*F4?-HcMA>1lAVKf|aqh<5eRTCyGs8F|`l>}dv0gnMU_Rcb0PQD6yS0xuX~(&Oy|Hg@NxnOoI~ija2Sp#Mjb zf9b7Kc}Qk|h-46a0+@u)KQ}kKH&l6sB4&>rG#*)Tt!YJ7{r0N{Fy%FxCtStb)K;8^ zt4NyUi-uQt$8+-7Pd@s)(TcqSE8; z@BYisz64lkU9&#qU3~A}h;DFW83S@B=eJWdwsGN(O;0F^)w15s%Ce@J4kM`Qn`A KCs<1fN&h#cz;@aI literal 0 HcmV?d00001 From 6dba54e8dbf5cda59394a8ab6261c9aff1e4a1f1 Mon Sep 17 00:00:00 2001 From: Donny Winston Date: Mon, 22 Jan 2024 13:38:31 -0500 Subject: [PATCH 23/23] style: improve docs for orcid login --- nmdc_runtime/api/main.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nmdc_runtime/api/main.py b/nmdc_runtime/api/main.py index e2107595..d403b607 100644 --- a/nmdc_runtime/api/main.py +++ b/nmdc_runtime/api/main.py @@ -369,6 +369,8 @@ async def get_versions(): f'Login with ORCiD' + " (note: this link is static; if you are logged in, you will see a 'locked' lock icon" + " in the below-right 'Authorized' button.)" ), openapi_tags=tags_metadata, lifespan=lifespan,