Update Healthcheck logic (#3884)

Co-authored-by: SteveDMurphy <[email protected]>
Co-authored-by: Robert Keyser <[email protected]>

CHANGELOG.md

@@ -35,6 +35,7 @@ The types of changes are:
 ### Changed
 
 - Simplified the file structure for HTML DSR packages [#3848](https://github.com/ethyca/fides/pull/3848)
+- Simplified the database health check to improve `/health` performance [#3884](https://github.com/ethyca/fides/pull/3884)
 - Changed max width of form components in "system information" form tab [#3864](https://github.com/ethyca/fides/pull/3864)
 - Remove manual system selection screen [#3865](https://github.com/ethyca/fides/pull/3865)
 - System and integration identifiers are now auto-generated [#3868](https://github.com/ethyca/fides/pull/3868)

noxfiles/ci_nox.py

@@ -2,9 +2,9 @@
 from functools import partial
 from typing import Callable, Dict
 
+import nox
 from nox.command import CommandFailed
 
-import nox
 from constants_nox import (
     CONTAINER_NAME,
     IMAGE_NAME,

noxfiles/utils_nox.py

@@ -2,6 +2,7 @@
 from pathlib import Path
 
 import nox
+
 from constants_nox import COMPOSE_FILE_LIST
 from run_infrastructure import run_infrastructure
 

src/fides/api/api/v1/endpoints/health.py

@@ -80,11 +80,9 @@ def get_cache_health() -> str:
         },
     },
 )
-async def health(
-    db: Session = Depends(get_db),
-) -> Dict:  # Intentionally injecting the ops get_db
+async def health() -> Dict:
     """Confirm that the API is running and healthy."""
-    database_health = get_db_health(CONFIG.database.sync_database_uri, db=db)
+    database_health = get_db_health(CONFIG.database.sync_database_uri)
     cache_health = get_cache_health()
     response = CoreHealthCheck(
         webserver="healthy",
@@ -97,6 +95,7 @@ async def health(
     fides_is_using_workers = not celery_app.conf["task_always_eager"]
     if fides_is_using_workers:
         response["workers_enabled"] = True
+        # Figure out a way to make this faster
         response["workers"] = get_worker_ids()
 
     for _, value in response.items():

src/fides/api/db/database.py

@@ -4,11 +4,10 @@
 from os import path
 from typing import Literal
 
-from alembic import command, script
+from alembic import command
 from alembic.config import Config
 from alembic.runtime import migration
 from loguru import logger as log
-from sqlalchemy.orm import Session
 from sqlalchemy_utils.functions import create_database, database_exists
 from sqlalchemy_utils.types.encrypted.encrypted_type import InvalidCiphertextError
 
@@ -18,7 +17,7 @@
 from fides.api.util.errors import get_full_exception_name
 from fides.core.utils import get_db_engine
 
-DatabaseHealth = Literal["healthy", "unhealthy", "needs migration"]
+DatabaseHealth = Literal["healthy", "unhealthy"]
 
 
 def get_alembic_config(database_url: str) -> Config:
@@ -82,23 +81,16 @@ def reset_db(database_url: str) -> None:
     log.info("Reset complete.")
 
 
-def get_db_health(database_url: str, db: Session) -> DatabaseHealth:
-    """Checks if the db is reachable and up to date in alembic migrations"""
+def get_db_health(database_url: str) -> DatabaseHealth:
+    """Checks if the db is reachable."""
     try:
-        alembic_config = get_alembic_config(database_url)
-        alembic_script_directory = script.ScriptDirectory.from_config(alembic_config)
-        context = migration.MigrationContext.configure(db.connection())
-
-        if (
-            context.get_current_revision()
-            != alembic_script_directory.get_current_head()
-        ):
-            return "needs migration"
-        return "healthy"
+        get_db_engine(database_url)
     except Exception as error:  # pylint: disable=broad-except
         error_type = get_full_exception_name(error)
         log.error("Unable to reach the database: {}: {}", error_type, error)
         return "unhealthy"
+    else:
+        return "healthy"
 
 
 async def configure_db(database_url: str, samples: bool = False) -> None:

src/fides/api/tasks/__init__.py

@@ -78,7 +78,7 @@ def _create_celery(config: FidesConfig = CONFIG) -> Celery:
 
 def get_worker_ids() -> List[Optional[str]]:
     """
-    Returns a list of the connected heahtly worker UUIDs.
+    Returns a list of the connected healthy worker UUIDs.
     """
     try:
         connected_workers = [

src/fides/api/util/connection_util.py

@@ -1,4 +1,4 @@
-from typing import List, Optional
+from typing import List, Optional, Union
 
 from fastapi import Depends, HTTPException
 from fideslang.validation import FidesKey
@@ -325,7 +325,7 @@ def connection_status(
 
     connector = get_connector(connection_config)
     try:
-        status: ConnectionTestStatus | None = connector.test_connection()
+        status: Union[ConnectionTestStatus, None] = connector.test_connection()
 
     except (ConnectionException, ClientUnsuccessfulException) as exc:
         logger.warning(

tests/ctl/core/test_api.py

@@ -2009,7 +2009,7 @@ def test_404_on_api_routes(test_config: FidesConfig) -> None:
 @pytest.mark.integration
 @pytest.mark.parametrize(
     "database_health, expected_status_code",
-    [("healthy", 200), ("needs migration", 200), ("unhealthy", 503)],
+    [("healthy", 200), ("unhealthy", 503)],
 )
 def test_api_ping(
     test_config: FidesConfig,
@@ -2018,7 +2018,7 @@ def test_api_ping(
     monkeypatch: MonkeyPatch,
     test_client: TestClient,
 ) -> None:
-    def mock_get_db_health(url: str, db) -> str:
+    def mock_get_db_health(url: str) -> str:
         return database_health
 
     monkeypatch.setattr(health, "get_db_health", mock_get_db_health)