[Unified Fides] Reduce Idle Health Check Connections (#1182)

* Stop creating new engines as part of the process of running health checks which cause us to have too many idle connections opened against the application database.
Use the same engine that is being shared across the "ops" API endpoints.

- Remove unused get_db_for_health_check

* Remove the ctl get_db used in the endpoints in favor of the ops get_db.

* Mock get_db_health needs two parameters.

src/fides/api/ctl/database/database.py

@@ -9,7 +9,7 @@
 from fideslang import DEFAULT_TAXONOMY
 from fideslib.db.base import Base
 from loguru import logger as log
-from sqlalchemy import create_engine
+from sqlalchemy.orm import Session
 from sqlalchemy_utils.functions import create_database, database_exists
 
 from fides.api.ctl.sql_models import sql_model_map
@@ -127,19 +127,18 @@ def reset_db(database_url: str) -> None:
         version.drop(connection)
 
 
-def get_db_health(database_url: str) -> str:
+def get_db_health(database_url: str, db: Session) -> str:
     """Checks if the db is reachable and up to date in alembic migrations"""
     try:
-        engine = create_engine(database_url)
         alembic_config = get_alembic_config(database_url)
         alembic_script_directory = script.ScriptDirectory.from_config(alembic_config)
-        with engine.begin() as conn:
-            context = migration.MigrationContext.configure(conn)
-            if (
-                context.get_current_revision()
-                != alembic_script_directory.get_current_head()
-            ):
-                return "needs migration"
+        context = migration.MigrationContext.configure(db.connection())
+
+        if (
+            context.get_current_revision()
+            != alembic_script_directory.get_current_head()
+        ):
+            return "needs migration"
         return "healthy"
     except Exception as error:  # pylint: disable=broad-except
         error_type = get_full_exception_name(error)

src/fides/api/ctl/deps.py

@@ -1,6 +1,5 @@
 import json
 from datetime import datetime
-from typing import Generator
 
 from fastapi import Depends, Security
 from fastapi.security import SecurityScopes
@@ -15,26 +14,16 @@
 from fideslib.oauth.scopes import SCOPES
 from sqlalchemy.orm import Session
 
-from fides.api.ctl.database.session import sync_session
 from fides.api.ctl.routes.util import API_PREFIX
 from fides.api.ctl.sql_models import ClientDetail, FidesUser
+from fides.api.ops.api.deps import get_db
 from fides.ctl.core.config import get_config
 
 oauth2_scheme = OAuth2ClientCredentialsBearer(
     tokenUrl=(f"{API_PREFIX}/oauth/token"),
 )
 
 
-def get_db() -> Generator:
-    """Return our database session"""
-
-    try:
-        db = sync_session()  # pylint: disable=invalid-name
-        yield db
-    finally:
-        db.close()
-
-
 async def verify_oauth_client(  # pylint: disable=invalid-name
     security_scopes: SecurityScopes,
     authorization: str = Security(oauth2_scheme),

src/fides/api/ctl/routes/health.py

@@ -1,12 +1,14 @@
 import logging
 from typing import Dict
 
-from fastapi import HTTPException, status
+from fastapi import Depends, HTTPException, status
 from redis.exceptions import ResponseError
+from sqlalchemy.orm import Session
 
 import fides
 from fides.api.ctl.database.database import get_db_health
 from fides.api.ctl.utils.api_router import APIRouter
+from fides.api.ops.api.deps import get_db
 from fides.api.ops.common_exceptions import RedisConnectionError
 from fides.api.ops.util.cache import get_cache
 from fides.api.ops.util.logger import Pii
@@ -66,9 +68,11 @@ def get_cache_health() -> str:
         },
     },
 )
-async def health() -> Dict:
+async def health(
+    db: Session = Depends(get_db),
+) -> Dict:  # Intentionally injecting the ops get_db
     """Confirm that the API is running and healthy."""
-    database_health = get_db_health(CONFIG.database.sync_database_uri)
+    database_health = get_db_health(CONFIG.database.sync_database_uri, db)
     cache_health = get_cache_health()
     response = {
         "webserver": "healthy",

src/fides/api/ops/api/deps.py

@@ -26,17 +26,8 @@ def get_db() -> Generator:
         db.close()
 
 
-def get_db_for_health_check() -> Generator:
-    """Gets a database session regardless of whether the application db is disabled, for a health check."""
-    try:
-        db = _get_session()
-        yield db
-    finally:
-        db.close()
-
-
 def _get_session() -> Session:
-    """Gets a database session"""
+    """Gets a database session, using the global engine if defined"""
     global _engine  # pylint: disable=W0603
     if not _engine:
         _engine = get_db_engine(config=CONFIG)

tests/ctl/core/test_api.py

@@ -16,8 +16,6 @@
 
 TAXONOMY_ENDPOINTS = ["data_category", "data_subject", "data_use", "data_qualifier"]
 
-TAXONOMY_ENDPOINTS = ["data_category", "data_subject", "data_use", "data_qualifier"]
-
 
 # Helper Functions
 def get_existing_key(test_config: FidesConfig, resource_type: str) -> int:
@@ -350,7 +348,7 @@ def test_api_ping(
     monkeypatch: MonkeyPatch,
     test_client: TestClient,
 ) -> None:
-    def mock_get_db_health(url: str) -> str:
+    def mock_get_db_health(url: str, db) -> str:
         return database_health
 
     monkeypatch.setattr(health, "get_db_health", mock_get_db_health)