Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make webserver resilient to database not starting up #649

Merged
merged 12 commits into from
May 19, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ The types of changes are:
* Resolved a failure with populating applicable data subject rights to a data map
* Updated `fideslog` to v1.1.5, resolving an issue where some exceptions thrown by the SDK were not handled as expected
* Host static files via fidesapi [#621](https://github.com/ethyca/fides/pull/621)
* Updated the webserver so that it won't fail if the database is inaccessible

## [1.6.0](https://github.com/ethyca/fides/compare/1.5.3...1.6.0) - 2022-05-02

Expand Down
30 changes: 28 additions & 2 deletions src/fidesapi/database/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,21 @@
"""
from os import path

from alembic import command
from alembic import command, script
from alembic.config import Config
from alembic.migration import MigrationContext
from alembic.runtime import migration
from fideslang import DEFAULT_TAXONOMY
from loguru import logger as log
from sqlalchemy import create_engine
from sqlalchemy_utils.functions import create_database, database_exists

from fidesapi.sql_models import SqlAlchemyBase, sql_model_map
from fidesapi.utils.errors import AlreadyExistsError, QueryError
from fidesapi.utils.errors import (
AlreadyExistsError,
QueryError,
get_full_exception_name,
)
from fidesctl.core.utils import get_db_engine

from .crud import create_resource, upsert_resources
Expand Down Expand Up @@ -108,3 +114,23 @@ def reset_db(database_url: str) -> None:
version = migration_context._version # pylint: disable=protected-access
if version.exists(connection):
version.drop(connection)


def get_db_health(database_url: str) -> 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"
return "healthy"
except Exception as error:
error_type = get_full_exception_name(error)
log.error(f"Unable to reach the database: {error_type}: {error}")
ThomasLaPiana marked this conversation as resolved.
Show resolved Hide resolved
return "unhealthy"
43 changes: 38 additions & 5 deletions src/fidesapi/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Contains the code that sets up the API.
"""


from datetime import datetime
from enum import Enum
from logging import WARNING
Expand All @@ -17,8 +18,10 @@
import fidesctl
from fidesapi import view
from fidesapi.database import database
from fidesapi.database.database import get_db_health
from fidesapi.routes import crud, visualize
from fidesapi.routes.util import API_PREFIX, WEBAPP_DIRECTORY, WEBAPP_INDEX
from fidesapi.utils.errors import get_full_exception_name
from fidesapi.utils.logger import setup as setup_logging
from fidesctl.core.config import FidesctlConfig, get_config

Expand All @@ -42,8 +45,12 @@ def configure_routes() -> None:

async def configure_db(database_url: str) -> None:
"Set up the db to be used by the app."
database.create_db_if_not_exists(database_url)
await database.init_db(database_url)
try:
database.create_db_if_not_exists(database_url)
await database.init_db(database_url)
except Exception as error:
error_type = get_full_exception_name(error)
log.error(f"Unable to configure database: {error_type}: {error}")


@app.on_event("startup")
Expand Down Expand Up @@ -96,20 +103,44 @@ async def log_request(request: Request, call_next: Callable) -> Response:
"example": {
"status": "healthy",
"version": "1.0.0",
"database": "healthy",
}
}
}
},
status.HTTP_503_SERVICE_UNAVAILABLE: {
"content": {
"application/json": {
"example": {
"detail": {
"status": "healthy",
"version": "1.0.0",
"database": "unhealthy",
}
}
}
}
}
},
},
tags=["Health"],
)
async def health() -> Dict:
"Confirm that the API is running and healthy."
return {
database_health = get_db_health(CONFIG.api.sync_database_url)
response = {
"status": "healthy",
"version": str(fidesctl.__version__),
"database": database_health,
}

for key in response:
if response[key] == "unhealthy":
raise HTTPException(
status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail=response
)

return response


class DBActions(str, Enum):
"The available path parameters for the `/admin/db/{action}` endpoint."
Expand Down Expand Up @@ -152,7 +183,9 @@ def read_other_paths(request: Request) -> FileResponse:

# raise 404 for anything that should be backend endpoint but we can't find it
if path.startswith(API_PREFIX[1:]):
raise HTTPException(status_code=404, detail="Item not found")
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND, detail="Item not found"
)

# otherwise return the index
return FileResponse(WEBAPP_INDEX)
Expand Down
10 changes: 10 additions & 0 deletions src/fidesapi/utils/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,13 @@ def __init__(self) -> None:
status.HTTP_500_INTERNAL_SERVER_ERROR,
detail={"error": "a database query failed"},
)


def get_full_exception_name(exception: Exception) -> str:
"""Get the full exception name
i.e. get sqlalchemy.exc.IntegrityError instead of just IntegrityError
"""
module = exception.__class__.__module__
if module is None or module == str.__class__.__module__:
return exception.__class__.__name__
return module + "." + exception.__class__.__name__
24 changes: 19 additions & 5 deletions tests/core/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
import pytest
import requests
from fideslang import model_list, parse
from pytest import MonkeyPatch

from fidesapi import main
from fidesapi.routes.util import API_PREFIX
from fidesctl.core import api as _api
from fidesctl.core.config import FidesctlConfig
Expand Down Expand Up @@ -49,11 +51,23 @@ def test_generate_resource_urls_with_id(test_config: FidesctlConfig) -> None:

# Integration Tests
@pytest.mark.integration
def test_api_ping(test_config: FidesctlConfig) -> None:
assert (
_api.ping(test_config.cli.server_url + API_PREFIX + "/health").status_code
== 200
)
@pytest.mark.parametrize(
"database_health, expected_status_code",
[("healthy", 200), ("needs migration", 200), ("unhealthy", 503)],
)
def test_api_ping(
test_config: FidesctlConfig,
database_health: str,
expected_status_code: int,
monkeypatch: MonkeyPatch,
) -> None:
def mock_get_db_health() -> str:
return database_health

# not working :(
ThomasLaPiana marked this conversation as resolved.
Show resolved Hide resolved
allisonking marked this conversation as resolved.
Show resolved Hide resolved
monkeypatch.setattr(main, "get_db_health", mock_get_db_health)
response = _api.ping(test_config.cli.server_url + API_PREFIX + "/health")
assert response.status_code == expected_status_code


@pytest.mark.integration
Expand Down