-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
#283: Improve health endpoint with missing required conditions #291
Changes from all commits
a9e54ab
632c0ad
befa99c
5e1a952
016a261
44f320f
06a12a7
43ba302
39fbaa9
60364d9
e22d489
8acfb53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,11 @@ | |
from pymongo.collection import Collection | ||
|
||
from app.common.app_schema import AppEnvironmentMode | ||
from app.database.database_schema import BaseDatabaseConnection, DatabaseCollection | ||
from app.database.database_schema import ( | ||
BaseDatabaseConnection, | ||
DatabaseCollection, | ||
DatabasePingFailedException, | ||
) | ||
from app.database.DatabaseProductionConnection import DatabaseProductionConnection | ||
from app.database.DatabaseTestingConnection import DatabaseTestingConnection | ||
from app.logging.logging_constants import LOGGING_DATABASE_MANAGER | ||
|
@@ -41,15 +45,38 @@ def get_collection_connection(cls, collection_name: DatabaseCollection) -> Colle | |
def init_database_connection( | ||
cls, environment: AppEnvironmentMode, connection_uri: str | ||
) -> None: | ||
"""Initializes the database connection and loads its unique instance\ | ||
based on current environment value | ||
""" | ||
Initializes the database connection. | ||
|
||
Args: | ||
environment (AppEnvironmentMode): the current environment value | ||
connection_uri (str): the database connection uri | ||
environment (AppEnvironmentMode): The current environment mode. | ||
connection_uri (str): The URI for connecting to the database. | ||
|
||
Returns: | ||
None | ||
""" | ||
from pymongo import MongoClient | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The purpose of this class is not having any initialization of the Database until we know if tests will be run or we're in production mode. |
||
|
||
database_connection_class = cls.database_connection_mapping.get( | ||
environment, DatabaseProductionConnection | ||
) | ||
database_connection_class.init_connection(connection_uri) | ||
cls.connection = database_connection_class | ||
cls.client = MongoClient(connection_uri) | ||
|
||
@classmethod | ||
def ping_database(cls) -> bool: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should move this to the concrete implementations of database connection, such as Prod and Testing, included in the same folder |
||
"""Pings the database to check the connection | ||
|
||
Raises: | ||
DatabasePingFailedException: if ping has failed | ||
|
||
Returns: | ||
bool: True if the database is reachable, False otherwise | ||
""" | ||
try: | ||
cls.client.admin.command("ping") | ||
except DatabasePingFailedException: | ||
return False | ||
else: | ||
return True |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,3 +75,4 @@ | |
|
||
# Utilities | ||
LOGGING_AUDIO_MANAGEMENT_UTILS = "AUDIO_MANAGEMENT_UTILS" | ||
LOGGING_HEALTH = "HEALTH" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,19 +3,119 @@ | |
""" | ||
|
||
from fastapi import APIRouter | ||
from fastapi.responses import Response | ||
from starlette.status import HTTP_200_OK | ||
from fastapi.responses import JSONResponse | ||
from starlette.status import HTTP_200_OK, HTTP_503_SERVICE_UNAVAILABLE | ||
|
||
from app.auth.auth_schema import AuthConfig | ||
from app.database.database_schema import DatabasePingFailedException | ||
from app.database.DatabaseConnectionManager import DatabaseConnectionManager | ||
from app.logging.logging_constants import LOGGING_HEALTH | ||
AntonioMrtz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
from app.logging.logging_schema import SpotifyElectronLogger | ||
from app.spotify_electron.health.health_schema import HealthCheckResponse | ||
from app.spotify_electron.song.providers.song_service_provider import SongServiceProvider | ||
|
||
router = APIRouter(prefix="/health", tags=["health"]) | ||
logger = SpotifyElectronLogger(LOGGING_HEALTH).getLogger() | ||
|
||
|
||
def check_database_connection() -> dict[str, str]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can return None if the |
||
"""Validates ping to database | ||
|
||
Returns: | ||
dict[str, str]: Dictionary with status and explanatory message | ||
""" | ||
try: | ||
if DatabaseConnectionManager.ping_database(): | ||
return {"status": "healthy", "message": "Database connection is active"} | ||
except DatabasePingFailedException: | ||
logger.exception(DatabasePingFailedException.ERROR) | ||
else: | ||
return {"status": "unhealthy", "message": DatabasePingFailedException.ERROR} | ||
|
||
|
||
def check_song_service() -> dict[str, str]: | ||
"""Validates if SongServiceProvider is inited | ||
|
||
Returns: | ||
dict[str, str]: Dictionary with status and explanatory message | ||
""" | ||
try: | ||
service = SongServiceProvider.get_song_service() | ||
except Exception as exception: | ||
logger.exception("SongServiceProvider health check failed") | ||
return { | ||
"status": "unhealthy", | ||
"message": f"SongServiceProvider check failed: {str(exception)}", | ||
} | ||
else: | ||
if service is None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be done in a single return x if y else z statement |
||
return {"status": "unhealthy", "message": "SongServiceProvider is not initialized"} | ||
return {"status": "healthy", "message": "SongServiceProvider is properly initialized"} | ||
|
||
|
||
@router.get("/", summary="Health Check Endpoint") | ||
def get_health() -> Response: | ||
"""Validates if the app has launched correctly | ||
def check_auth_config() -> dict[str, str]: | ||
"""Validates if auth configurations are set | ||
|
||
Returns | ||
------- | ||
Response 200 OK | ||
Returns: | ||
dict[str, str]: Dictionary with status and explanatory message | ||
""" | ||
try: | ||
if not all( | ||
[ | ||
hasattr(AuthConfig, "VERTIFICATION_ALGORITHM") | ||
and AuthConfig.VERTIFICATION_ALGORITHM, | ||
hasattr(AuthConfig, "ACCESS_TOKEN_EXPIRE_MINUTES") | ||
and AuthConfig.ACCESS_TOKEN_EXPIRE_MINUTES, | ||
hasattr(AuthConfig, "DAYS_TO_EXPIRE_COOKIE") | ||
and AuthConfig.DAYS_TO_EXPIRE_COOKIE, | ||
] | ||
): | ||
return { | ||
"status": "unhealthy", | ||
"message": "Auth configuration is not fully initialized", | ||
} | ||
except Exception: | ||
logger.exception("Auth configuration health check failed") | ||
return { | ||
"status": "unhealthy", | ||
"message": "Auth configuration check failed", | ||
} | ||
else: | ||
return {"status": "healthy", "message": "Auth configuration is properly initialized"} | ||
|
||
|
||
@router.get( | ||
"/", | ||
response_model=HealthCheckResponse, | ||
summary="Health Check Endpoint", | ||
description="Validates if the app and its critical components are functioning correctly", | ||
) | ||
async def get_health() -> JSONResponse: | ||
"""Validates if the app has launched correctly and all critical components are healthy | ||
|
||
Returns: | ||
JSONResponse: health status with details of each component and status code | ||
""" | ||
return Response(status_code=HTTP_200_OK, content="OK", media_type="text/plain") | ||
db_health = check_database_connection() | ||
service_health = check_song_service() | ||
auth_health = check_auth_config() | ||
|
||
health_details = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a class |
||
"database": db_health, | ||
"song_service": service_health, | ||
"auth_config": auth_health, | ||
} | ||
|
||
is_healthy = all(check["status"] == "healthy" for check in health_details.values()) | ||
|
||
response_data = { | ||
"status": "healthy" if is_healthy else "unhealthy", | ||
"details": health_details, | ||
} | ||
|
||
status_code = HTTP_200_OK if is_healthy else HTTP_503_SERVICE_UNAVAILABLE | ||
|
||
if not is_healthy: | ||
logger.warning(f"Health check failed: {health_details}") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dont think this is needed, we're going to return |
||
|
||
return JSONResponse(content=response_data, status_code=status_code) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
""" | ||
This module defines the schema for health check responses. | ||
|
||
It includes the `HealthCheckResponse` model, which represents the structure | ||
of responses for system health checks. | ||
""" | ||
|
||
from pydantic import BaseModel | ||
|
||
|
||
class HealthCheckResponse(BaseModel): | ||
"""Class that represents response of health check""" | ||
|
||
status: str | ||
details: dict[str, dict[str, str]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need to add docs for returning None. The extension njpwerner.autodocstring handles it like that and I think Pycharm too