From 49fced8e105020c8ca352e10d0c69311bd1fad24 Mon Sep 17 00:00:00 2001 From: KhushiRajurkar Date: Mon, 3 Feb 2025 21:24:02 +0530 Subject: [PATCH 01/11] Refactor main.py: Replace deprecated startup/shutdown events with lifespan events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Replaced deprecated startup and shutdown events with lifespan events. * Improved environment variable validation and vocabulary loading during app lifecycle. * Ensured compatibility with FastAPI’s modern event handling approach. --- app/main.py | 150 ++++++++++++++++++++++------------------------------ 1 file changed, 64 insertions(+), 86 deletions(-) diff --git a/app/main.py b/app/main.py index a794f7b..6fb9e9b 100644 --- a/app/main.py +++ b/app/main.py @@ -4,6 +4,7 @@ import warnings from pathlib import Path from tempfile import TemporaryDirectory +from contextlib import asynccontextmanager import uvicorn from fastapi import FastAPI, Request @@ -11,17 +12,75 @@ from fastapi.openapi.docs import get_redoc_html, get_swagger_ui_html from fastapi.responses import HTMLResponse, ORJSONResponse, RedirectResponse -from .api import utility as util -from .api.routers import assessments, attributes, diagnoses, pipelines, query -from .api.security import check_client_id +from app.api import utility as util +from app.api.routers import assessments, attributes, diagnoses, pipelines, query +from app.api.security import check_client_id + + +def validate_environment_variables(): + """Validate required environment variables.""" + if os.environ.get(util.GRAPH_USERNAME.name) is None or os.environ.get(util.GRAPH_PASSWORD.name) is None: + raise RuntimeError( + f"The application was launched but could not find the {util.GRAPH_USERNAME.name} and/or {util.GRAPH_PASSWORD.name} environment variables." + ) + + if os.environ.get(util.ALLOWED_ORIGINS.name, "") == "": + warnings.warn( + f"The API was launched without providing any values for the {util.ALLOWED_ORIGINS.name} environment " + f"variable." + "This means that the API will only be accessible from the same origin it is hosted from: " + "https://developer.mozilla.org/en-US/docs/Web/Security/Same-origin_policy." + f"If you want to access the API from tools hosted at other origins such as the Neurobagel query tool, " + f"explicitly set the value of {util.ALLOWED_ORIGINS.name} to the origin(s) of these tools (e.g. " + f"http://localhost:3000)." + "Multiple allowed origins should be separated with spaces in a single string enclosed in quotes." + ) + + +@asynccontextmanager +async def lifespan(app: FastAPI): + """Lifespan event handler for startup and shutdown logic.""" + try: + # Validate environment variables + validate_environment_variables() + + # Authentication check + check_client_id() + + # Create and store temporary directories + app.state.vocab_dir = TemporaryDirectory() + app.state.vocab_dir_path = Path(app.state.vocab_dir.name) + + app.state.vocab_lookup_paths = { + "snomed_assessment": app.state.vocab_dir_path / "snomedct_assessment_term_labels.json", + "snomed_disorder": app.state.vocab_dir_path / "snomedct_disorder_term_labels.json" + } + + # Create vocabulary lookups + util.create_snomed_assessment_lookup(app.state.vocab_lookup_paths["snomed_assessment"]) + util.create_snomed_disorder_lookup(app.state.vocab_lookup_paths["snomed_disorder"]) + + except Exception as e: + raise RuntimeError(f"Startup failed: {str(e)}") + + yield + + # Shutdown logic + try: + app.state.vocab_dir.cleanup() + except Exception as e: + warnings.warn(f"Failed to clean up temporary directory: {str(e)}") + app = FastAPI( root_path=util.ROOT_PATH.val, + lifespan=lifespan, default_response_class=ORJSONResponse, docs_url=None, redoc_url=None, redirect_slashes=False, ) + favicon_url = "https://raw.githubusercontent.com/neurobagel/documentation/main/docs/imgs/logo/neurobagel_favicon.png" app.add_middleware( @@ -35,32 +94,21 @@ @app.get("/", response_class=HTMLResponse) def root(request: Request): - """ - Display a welcome message and a link to the API documentation. - """ return f"""

Welcome to the Neurobagel REST API!

-

Please visit the API documentation to view available API endpoints.

- - - """ +

Please visit the API documentation to view + available API endpoints.

""" @app.get("/favicon.ico", include_in_schema=False) async def favicon(): - """ - Overrides the default favicon with a custom one. - """ return RedirectResponse(url=favicon_url) @app.get("/docs", include_in_schema=False) def overridden_swagger(request: Request): - """ - Overrides the Swagger UI HTML for the "/docs" endpoint. - """ return get_swagger_ui_html( openapi_url=f"{request.scope.get('root_path', '')}/openapi.json", title="Neurobagel API", @@ -70,9 +118,6 @@ def overridden_swagger(request: Request): @app.get("/redoc", include_in_schema=False) def overridden_redoc(request: Request): - """ - Overrides the Redoc HTML for the "/redoc" endpoint. - """ return get_redoc_html( openapi_url=f"{request.scope.get('root_path', '')}/openapi.json", title="Neurobagel API", @@ -80,78 +125,11 @@ def overridden_redoc(request: Request): ) -@app.on_event("startup") -async def auth_check(): - """ - Checks whether authentication has been enabled for API queries and whether the - username and password environment variables for the graph backend have been set. - - TODO: Refactor once startup events have been replaced by lifespan event - """ - check_client_id() - - if ( - # TODO: Check if this error is still raised when variables are empty strings - os.environ.get(util.GRAPH_USERNAME.name) is None - or os.environ.get(util.GRAPH_PASSWORD.name) is None - ): - raise RuntimeError( - f"The application was launched but could not find the {util.GRAPH_USERNAME.name} and / or {util.GRAPH_PASSWORD.name} environment variables." - ) - - -@app.on_event("startup") -async def allowed_origins_check(): - """Raises warning if allowed origins environment variable has not been set or is an empty string.""" - if os.environ.get(util.ALLOWED_ORIGINS.name, "") == "": - warnings.warn( - f"The API was launched without providing any values for the {util.ALLOWED_ORIGINS.name} environment variable. " - "This means that the API will only be accessible from the same origin it is hosted from: https://developer.mozilla.org/en-US/docs/Web/Security/Same-origin_policy. " - f"If you want to access the API from tools hosted at other origins such as the Neurobagel query tool, explicitly set the value of {util.ALLOWED_ORIGINS.name} to the origin(s) of these tools (e.g. http://localhost:3000). " - "Multiple allowed origins should be separated with spaces in a single string enclosed in quotes. " - ) - - -@app.on_event("startup") -async def fetch_vocabularies_to_temp_dir(): - """ - Create and store on the app instance a temporary directory for vocabulary term lookup JSON files - (each of which contain key-value pairings of IDs to human-readable names of terms), - and then fetch vocabularies using their respective native APIs and save them to the temporary directory for reuse. - """ - # We use Starlette's ability (FastAPI is Starlette underneath) to store arbitrary state on the app instance (https://www.starlette.io/applications/#storing-state-on-the-app-instance) - # to store a temporary directory object and its corresponding path. These data are local to the instance and will be recreated on every app launch (i.e. not persisted). - app.state.vocab_dir = TemporaryDirectory() - app.state.vocab_dir_path = Path(app.state.vocab_dir.name) - - app.state.vocab_lookup_paths = {} - app.state.vocab_lookup_paths["snomed_assessment"] = ( - app.state.vocab_dir_path / "snomedct_assessment_term_labels.json" - ) - app.state.vocab_lookup_paths["snomed_disorder"] = ( - app.state.vocab_dir_path / "snomedct_disorder_term_labels.json" - ) - - util.create_snomed_assessment_lookup( - app.state.vocab_lookup_paths["snomed_assessment"] - ) - util.create_snomed_disorder_lookup( - app.state.vocab_lookup_paths["snomed_disorder"] - ) - - -@app.on_event("shutdown") -async def cleanup_temp_vocab_dir(): - """Clean up the temporary directory created on startup.""" - app.state.vocab_dir.cleanup() - - app.include_router(query.router) app.include_router(attributes.router) app.include_router(assessments.router) app.include_router(diagnoses.router) app.include_router(pipelines.router) -# Automatically start uvicorn server on execution of main.py if __name__ == "__main__": uvicorn.run("app.main:app", port=8000, reload=True) From 24ddc7856d8da78b3897fb79b1eb77e17fdf97e5 Mon Sep 17 00:00:00 2001 From: KhushiRajurkar Date: Tue, 18 Feb 2025 18:16:58 +0530 Subject: [PATCH 02/11] Refactor docstrings, fix spacing in function, and modify error type - Restored previously deleted docstrings - Fixed spacing issues in the `validate_environment_variables` - Changed raised error from `ValueError` to `RuntimeError` in `security.py` - Updated corresponding assertion in `test_security.py` --- app/api/security.py | 2 +- app/main.py | 94 +++++++++++++++++++++++++++++------------- tests/test_security.py | 4 +- 3 files changed, 69 insertions(+), 31 deletions(-) diff --git a/app/api/security.py b/app/api/security.py index 9f38464..551bf92 100644 --- a/app/api/security.py +++ b/app/api/security.py @@ -22,7 +22,7 @@ def check_client_id(): """Check if the CLIENT_ID environment variable is set.""" # The CLIENT_ID is needed to verify the audience claim of ID tokens. if AUTH_ENABLED and CLIENT_ID is None: - raise ValueError( + raise RuntimeError( "Authentication has been enabled (NB_ENABLE_AUTH) but the environment variable NB_QUERY_CLIENT_ID is not set. " "Please set NB_QUERY_CLIENT_ID to the client ID for your Neurobagel query tool deployment, to verify the audience claim of ID tokens." ) diff --git a/app/main.py b/app/main.py index 6fb9e9b..c646b70 100644 --- a/app/main.py +++ b/app/main.py @@ -2,9 +2,9 @@ import os import warnings +from contextlib import asynccontextmanager from pathlib import Path from tempfile import TemporaryDirectory -from contextlib import asynccontextmanager import uvicorn from fastapi import FastAPI, Request @@ -18,10 +18,20 @@ def validate_environment_variables(): - """Validate required environment variables.""" - if os.environ.get(util.GRAPH_USERNAME.name) is None or os.environ.get(util.GRAPH_PASSWORD.name) is None: + """ + Check that all required environment variables are set. + + Ensures that the username and password for the graph database are provided. + If not, raises a RuntimeError to prevent the application from running without valid credentials. + + Also checks that ALLOWED_ORIGINS is properly set. If missing, a warning is issued, but the app continues running. + """ + if ( + os.environ.get(util.GRAPH_USERNAME.name) is None + or os.environ.get(util.GRAPH_PASSWORD.name) is None + ): raise RuntimeError( - f"The application was launched but could not find the {util.GRAPH_USERNAME.name} and/or {util.GRAPH_PASSWORD.name} environment variables." + f"The application was launched but could not find the {util.GRAPH_USERNAME.name} and / or {util.GRAPH_PASSWORD.name} environment variables." ) if os.environ.get(util.ALLOWED_ORIGINS.name, "") == "": @@ -37,39 +47,60 @@ def validate_environment_variables(): ) +def initialize_vocabularies(app): + """ + Create and store on the app instance a temporary directory for vocabulary term lookup JSON files + (each of which contain key-value pairings of IDs to human-readable names of terms), + and then fetch vocabularies using their respective native APIs and save them to the temporary directory for reuse. + """ + # We use Starlette's ability (FastAPI is Starlette underneath) to store arbitrary state on the app instance (https://www.starlette.io/applications/#storing-state-on-the-app-instance) + # to store a temporary directory object and its corresponding path. These data are local to the instance and will be recreated on every app launch (i.e. not persisted). + + app.state.vocab_dir = TemporaryDirectory() + app.state.vocab_dir_path = Path(app.state.vocab_dir.name) + + app.state.vocab_lookup_paths = { + "snomed_assessment": app.state.vocab_dir_path + / "snomedct_assessment_term_labels.json", + "snomed_disorder": app.state.vocab_dir_path + / "snomedct_disorder_term_labels.json", + } + + util.create_snomed_assessment_lookup( + app.state.vocab_lookup_paths["snomed_assessment"] + ) + util.create_snomed_disorder_lookup( + app.state.vocab_lookup_paths["snomed_disorder"] + ) + + + @asynccontextmanager async def lifespan(app: FastAPI): - """Lifespan event handler for startup and shutdown logic.""" - try: - # Validate environment variables - validate_environment_variables() - - # Authentication check - check_client_id() + """ + Handles application startup and shutdown events. - # Create and store temporary directories - app.state.vocab_dir = TemporaryDirectory() - app.state.vocab_dir_path = Path(app.state.vocab_dir.name) + On startup: + - Validates required environment variables. + - Performs authentication checks. + - Initializes temporary directories for vocabulary lookups. - app.state.vocab_lookup_paths = { - "snomed_assessment": app.state.vocab_dir_path / "snomedct_assessment_term_labels.json", - "snomed_disorder": app.state.vocab_dir_path / "snomedct_disorder_term_labels.json" - } + On shutdown: + - Cleans up temporary directories to free resources. + """ + # Validate environment variables + validate_environment_variables() - # Create vocabulary lookups - util.create_snomed_assessment_lookup(app.state.vocab_lookup_paths["snomed_assessment"]) - util.create_snomed_disorder_lookup(app.state.vocab_lookup_paths["snomed_disorder"]) + # Authentication check + check_client_id() - except Exception as e: - raise RuntimeError(f"Startup failed: {str(e)}") + # Initialize vocabularies + initialize_vocabularies(app) yield # Shutdown logic - try: - app.state.vocab_dir.cleanup() - except Exception as e: - warnings.warn(f"Failed to clean up temporary directory: {str(e)}") + app.state.vocab_dir.cleanup() app = FastAPI( @@ -98,12 +129,19 @@ def root(request: Request):

Welcome to the Neurobagel REST API!

-

Please visit the API documentation to view +

Please visit the API documentation to view available API endpoints.

""" @app.get("/favicon.ico", include_in_schema=False) async def favicon(): + """ + Overrides the default favicon with a custom one. + + NOTE: When the API is behind a reverse proxy that has a stripped path prefix (and root_path is defined), + the custom favicon doesn't appear to work correctly for any API paths other than the docs, + as the path in the favicon request isn't automatically adjusted to include the root path prefix. + """ return RedirectResponse(url=favicon_url) diff --git a/tests/test_security.py b/tests/test_security.py index 57cbe3e..ab827b9 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -14,11 +14,11 @@ def test_missing_client_id_raises_error_when_auth_enabled( # but we set the values explicitly here for clarity monkeypatch.setattr("app.api.security.CLIENT_ID", None) - with pytest.raises(ValueError) as exc_info: + with pytest.raises(RuntimeError) as exc_info: with test_app: pass - assert "NB_QUERY_CLIENT_ID is not set" in str(exc_info.value) + assert "Authentication has been enabled (NB_ENABLE_AUTH) but the environment variable NB_QUERY_CLIENT_ID is not set." in str(exc_info.value) @pytest.mark.filterwarnings("ignore:.*NB_API_ALLOWED_ORIGINS") From 4cc91c50d7c8aee8d35032d981fc6389989a555b Mon Sep 17 00:00:00 2001 From: KhushiRajurkar Date: Tue, 18 Feb 2025 18:19:42 +0530 Subject: [PATCH 03/11] Update main.py Added a comment that was previously deleted --- app/main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/app/main.py b/app/main.py index c646b70..5c62590 100644 --- a/app/main.py +++ b/app/main.py @@ -169,5 +169,6 @@ def overridden_redoc(request: Request): app.include_router(diagnoses.router) app.include_router(pipelines.router) +# Automatically start uvicorn server on execution of main.py if __name__ == "__main__": uvicorn.run("app.main:app", port=8000, reload=True) From 77ab58714aa40032ca180d9a7791fb866348b314 Mon Sep 17 00:00:00 2001 From: KhushiRajurkar Date: Tue, 18 Feb 2025 18:23:23 +0530 Subject: [PATCH 04/11] Update main.py Revert deletions of comments in `overridden_swagger` and `overridden_redoc` --- app/main.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/main.py b/app/main.py index 5c62590..828abcc 100644 --- a/app/main.py +++ b/app/main.py @@ -147,6 +147,9 @@ async def favicon(): @app.get("/docs", include_in_schema=False) def overridden_swagger(request: Request): + """ + Overrides the Swagger UI HTML for the "/docs" endpoint. + """ return get_swagger_ui_html( openapi_url=f"{request.scope.get('root_path', '')}/openapi.json", title="Neurobagel API", @@ -156,6 +159,9 @@ def overridden_swagger(request: Request): @app.get("/redoc", include_in_schema=False) def overridden_redoc(request: Request): + """ + Overrides the Redoc HTML for the "/redoc" endpoint. + """ return get_redoc_html( openapi_url=f"{request.scope.get('root_path', '')}/openapi.json", title="Neurobagel API", From 15317ff1b3d9b4feea16e971b79c4f79d6beb6cc Mon Sep 17 00:00:00 2001 From: KhushiRajurkar Date: Wed, 19 Feb 2025 05:13:50 +0530 Subject: [PATCH 05/11] Update main.py Revert deletion of docstring in the `root` function --- app/main.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/main.py b/app/main.py index 828abcc..f333c69 100644 --- a/app/main.py +++ b/app/main.py @@ -125,6 +125,9 @@ async def lifespan(app: FastAPI): @app.get("/", response_class=HTMLResponse) def root(request: Request): + """ + Display a welcome message and a link to the API documentation. + """ return f""" From 76701f2ba79b3bb1642771245ec4b9990551d87b Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Wed, 19 Feb 2025 23:05:28 -0500 Subject: [PATCH 06/11] revert more detailed assertion --- tests/test_security.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_security.py b/tests/test_security.py index ab827b9..cd06de1 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -18,7 +18,7 @@ def test_missing_client_id_raises_error_when_auth_enabled( with test_app: pass - assert "Authentication has been enabled (NB_ENABLE_AUTH) but the environment variable NB_QUERY_CLIENT_ID is not set." in str(exc_info.value) + assert "NB_QUERY_CLIENT_ID is not set" in str(exc_info.value) @pytest.mark.filterwarnings("ignore:.*NB_API_ALLOWED_ORIGINS") From a2cca1d89a32a5b822f5c695371418d1b525d673 Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Wed, 19 Feb 2025 23:15:29 -0500 Subject: [PATCH 07/11] update pre-commit config so isort and black use same line length --- .pre-commit-config.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7f42006..69e78d9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -34,7 +34,10 @@ repos: rev: 5.13.2 hooks: - id: isort - args: ["--profile", "black", "--filter-files"] + args: + - "--profile=black" + - "--filter-files" + - "--line-length=79" - repo: https://github.com/codespell-project/codespell rev: v2.4.0 From 20c9af922508cc6b3b9e3ce28f4c05f9d0a8f24b Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Wed, 19 Feb 2025 23:15:42 -0500 Subject: [PATCH 08/11] remove unneeded function argument --- app/main.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/app/main.py b/app/main.py index f333c69..bf3f107 100644 --- a/app/main.py +++ b/app/main.py @@ -13,7 +13,13 @@ from fastapi.responses import HTMLResponse, ORJSONResponse, RedirectResponse from app.api import utility as util -from app.api.routers import assessments, attributes, diagnoses, pipelines, query +from app.api.routers import ( + assessments, + attributes, + diagnoses, + pipelines, + query, +) from app.api.security import check_client_id @@ -47,7 +53,7 @@ def validate_environment_variables(): ) -def initialize_vocabularies(app): +def initialize_vocabularies(): """ Create and store on the app instance a temporary directory for vocabulary term lookup JSON files (each of which contain key-value pairings of IDs to human-readable names of terms), @@ -74,7 +80,6 @@ def initialize_vocabularies(app): ) - @asynccontextmanager async def lifespan(app: FastAPI): """ @@ -95,7 +100,7 @@ async def lifespan(app: FastAPI): check_client_id() # Initialize vocabularies - initialize_vocabularies(app) + initialize_vocabularies() yield From 975e8c99c5343a0f2493acc7b4123269aead3c24 Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Wed, 19 Feb 2025 23:17:32 -0500 Subject: [PATCH 09/11] revert change to absolute imports --- app/main.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/app/main.py b/app/main.py index bf3f107..04bcd91 100644 --- a/app/main.py +++ b/app/main.py @@ -12,15 +12,9 @@ from fastapi.openapi.docs import get_redoc_html, get_swagger_ui_html from fastapi.responses import HTMLResponse, ORJSONResponse, RedirectResponse -from app.api import utility as util -from app.api.routers import ( - assessments, - attributes, - diagnoses, - pipelines, - query, -) -from app.api.security import check_client_id +from .api import utility as util +from .api.routers import assessments, attributes, diagnoses, pipelines, query +from .api.security import check_client_id def validate_environment_variables(): From b4e015aabb68b60a995b62a8830b0cea08b9b267 Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Wed, 19 Feb 2025 23:30:37 -0500 Subject: [PATCH 10/11] fix formatting of html string --- app/main.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/main.py b/app/main.py index 04bcd91..82c4af4 100644 --- a/app/main.py +++ b/app/main.py @@ -131,8 +131,9 @@ def root(request: Request):

Welcome to the Neurobagel REST API!

-

Please visit the API documentation to view - available API endpoints.

""" +

Please visit the API documentation to view available API endpoints.

+ + """ @app.get("/favicon.ico", include_in_schema=False) From 913bfa8ca8868830d6124e4471b0f878869bbf7d Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Wed, 19 Feb 2025 23:32:19 -0500 Subject: [PATCH 11/11] fix formatting of html string --- app/main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/main.py b/app/main.py index 82c4af4..1273115 100644 --- a/app/main.py +++ b/app/main.py @@ -133,7 +133,8 @@ def root(request: Request):

Welcome to the Neurobagel REST API!

Please visit the API documentation to view available API endpoints.

- """ + + """ @app.get("/favicon.ico", include_in_schema=False)