From 848befe3f18519b01325f93bc1467bbae3324d9d Mon Sep 17 00:00:00 2001 From: dettlaff Date: Wed, 23 Oct 2024 14:38:01 +0300 Subject: [PATCH] feat: Use proper logging (#154) Reviewed-on: https://git.selfprivacy.org/SelfPrivacy/selfprivacy-rest-api/pulls/154 Reviewed-by: Inex Code Co-authored-by: dettlaff Co-committed-by: dettlaff --- selfprivacy_api/app.py | 9 +++++++++ selfprivacy_api/graphql/queries/services.py | 1 + selfprivacy_api/jobs/migrate_to_binds.py | 9 ++++++--- selfprivacy_api/migrations/__init__.py | 10 +++++++--- .../migrations/write_token_to_redis.py | 15 +++++++++------ selfprivacy_api/services/__init__.py | 5 ++++- selfprivacy_api/services/generic_size_counter.py | 5 ++++- selfprivacy_api/services/owned_path.py | 11 ++++++++--- 8 files changed, 48 insertions(+), 17 deletions(-) diff --git a/selfprivacy_api/app.py b/selfprivacy_api/app.py index 2f7e2f7..703de18 100644 --- a/selfprivacy_api/app.py +++ b/selfprivacy_api/app.py @@ -1,5 +1,8 @@ #!/usr/bin/env python3 """SelfPrivacy server management API""" +import logging +import os + from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware from strawberry.fastapi import GraphQLRouter @@ -12,6 +15,12 @@ from selfprivacy_api.graphql.schema import schema from selfprivacy_api.migrations import run_migrations +log_level = os.getenv("LOG_LEVEL", "INFO").upper() + +logging.basicConfig( + level=getattr(logging, log_level, logging.INFO), format="%(levelname)s: %(message)s" +) + app = FastAPI() graphql_app: GraphQLRouter = GraphQLRouter( diff --git a/selfprivacy_api/graphql/queries/services.py b/selfprivacy_api/graphql/queries/services.py index 63e647d..2f95544 100644 --- a/selfprivacy_api/graphql/queries/services.py +++ b/selfprivacy_api/graphql/queries/services.py @@ -2,6 +2,7 @@ # pylint: disable=too-few-public-methods import typing + import strawberry from selfprivacy_api.graphql.common_types.service import ( diff --git a/selfprivacy_api/jobs/migrate_to_binds.py b/selfprivacy_api/jobs/migrate_to_binds.py index 7ab1cd5..bde60fb 100644 --- a/selfprivacy_api/jobs/migrate_to_binds.py +++ b/selfprivacy_api/jobs/migrate_to_binds.py @@ -3,6 +3,7 @@ import subprocess import pathlib import shutil +import logging from pydantic import BaseModel from selfprivacy_api.jobs import Job, JobStatus, Jobs @@ -15,6 +16,8 @@ from selfprivacy_api.utils import ReadUserData, WriteUserData from selfprivacy_api.utils.huey import huey from selfprivacy_api.utils.block_devices import BlockDevices +logger = logging.getLogger(__name__) + class BindMigrationConfig(BaseModel): """Config for bind migration. @@ -69,7 +72,7 @@ def move_folder( try: data_path.mkdir(mode=0o750, parents=True, exist_ok=True) except Exception as error: - print(f"Error creating data path: {error}") + logging.error(f"Error creating data path: {error}") return try: @@ -81,12 +84,12 @@ def move_folder( try: subprocess.run(["mount", "--bind", str(bind_path), str(data_path)], check=True) except subprocess.CalledProcessError as error: - print(error) + logging.error(error) try: subprocess.run(["chown", "-R", f"{user}:{group}", str(data_path)], check=True) except subprocess.CalledProcessError as error: - print(error) + logging.error(error) @huey.task() diff --git a/selfprivacy_api/migrations/__init__.py b/selfprivacy_api/migrations/__init__.py index 327bb28..24d2c3c 100644 --- a/selfprivacy_api/migrations/__init__.py +++ b/selfprivacy_api/migrations/__init__.py @@ -9,6 +9,8 @@ with IDs of the migrations to skip. Adding DISABLE_ALL to that array disables the migrations module entirely. """ +import logging + from selfprivacy_api.utils import ReadUserData, UserDataFiles from selfprivacy_api.migrations.write_token_to_redis import WriteTokenToRedis from selfprivacy_api.migrations.check_for_system_rebuild_jobs import ( @@ -17,6 +19,8 @@ from selfprivacy_api.migrations.check_for_system_rebuild_jobs import ( from selfprivacy_api.migrations.add_roundcube import AddRoundcube from selfprivacy_api.migrations.add_monitoring import AddMonitoring +logger = logging.getLogger(__name__) + migrations = [ WriteTokenToRedis(), CheckForSystemRebuildJobs(), @@ -47,6 +51,6 @@ def run_migrations(): if migration.is_migration_needed(): migration.migrate() except Exception as err: - print(f"Error while migrating {migration.get_migration_name()}") - print(err) - print("Skipping this migration") + logging.error(f"Error while migrating {migration.get_migration_name()}") + logging.error(err) + logging.error("Skipping this migration") diff --git a/selfprivacy_api/migrations/write_token_to_redis.py b/selfprivacy_api/migrations/write_token_to_redis.py index ccf1c04..a43313e 100644 --- a/selfprivacy_api/migrations/write_token_to_redis.py +++ b/selfprivacy_api/migrations/write_token_to_redis.py @@ -1,3 +1,4 @@ +import logging from datetime import datetime from typing import Optional from selfprivacy_api.migrations.migration import Migration @@ -11,6 +12,8 @@ from selfprivacy_api.repositories.tokens.abstract_tokens_repository import ( ) from selfprivacy_api.utils import ReadUserData, UserDataFiles +logger = logging.getLogger(__name__) + class WriteTokenToRedis(Migration): """Load Json tokens into Redis""" @@ -35,7 +38,7 @@ class WriteTokenToRedis(Migration): created_at=datetime.now(), ) except Exception as e: - print(e) + logging.error(e) return None def is_migration_needed(self) -> bool: @@ -45,7 +48,7 @@ class WriteTokenToRedis(Migration): ): return True except Exception as e: - print(e) + logging.error(e) return False return False @@ -54,11 +57,11 @@ class WriteTokenToRedis(Migration): try: token = self.get_token_from_json() if token is None: - print("No token found in secrets.json") + logging.error("No token found in secrets.json") return RedisTokensRepository()._store_token(token) - print("Done") + logging.error("Done") except Exception as e: - print(e) - print("Error migrating access tokens from json to redis") + logging.error(e) + logging.error("Error migrating access tokens from json to redis") diff --git a/selfprivacy_api/services/__init__.py b/selfprivacy_api/services/__init__.py index e829775..755e389 100644 --- a/selfprivacy_api/services/__init__.py +++ b/selfprivacy_api/services/__init__.py @@ -1,5 +1,6 @@ """Services module.""" +import logging import base64 import typing from typing import List @@ -30,6 +31,8 @@ from selfprivacy_api.utils import read_account_uri CONFIG_STASH_DIR = "/etc/selfprivacy/dump" +logger = logging.getLogger(__name__) + class ServiceManager(Service): folders: List[str] = [CONFIG_STASH_DIR] @@ -76,7 +79,7 @@ class ServiceManager(Service): ) ) except Exception as e: - print(f"Error creating CAA: {e}") + logging.error(f"Error creating CAA: {e}") for service in ServiceManager.get_enabled_services(): dns_records += service.get_dns_records(ip4, ip6) diff --git a/selfprivacy_api/services/generic_size_counter.py b/selfprivacy_api/services/generic_size_counter.py index 9bb6baa..a32f4dd 100644 --- a/selfprivacy_api/services/generic_size_counter.py +++ b/selfprivacy_api/services/generic_size_counter.py @@ -1,6 +1,9 @@ """Generic size counter using pathlib""" import pathlib +import logging + +logger = logging.getLogger(__name__) def get_storage_usage(path: str) -> int: @@ -18,5 +21,5 @@ def get_storage_usage(path: str) -> int: except FileNotFoundError: pass except Exception as error: - print(error) + logging.error(error) return storage_usage diff --git a/selfprivacy_api/services/owned_path.py b/selfprivacy_api/services/owned_path.py index aa6e92e..1bbe4df 100644 --- a/selfprivacy_api/services/owned_path.py +++ b/selfprivacy_api/services/owned_path.py @@ -1,11 +1,16 @@ from __future__ import annotations +import logging import subprocess import pathlib -from pydantic import BaseModel from os.path import exists +from pydantic import BaseModel + from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices + +logger = logging.getLogger(__name__) + # tests override it to a tmpdir VOLUMES_PATH = "/volumes" @@ -87,7 +92,7 @@ class Bind: check=True, ) except subprocess.CalledProcessError as error: - print(error.stderr) + logging.error(error.stderr) raise BindError(f"Unable to bind {source} to {target} :{error.stderr}") def unbind(self) -> None: @@ -119,7 +124,7 @@ class Bind: stderr=subprocess.PIPE, ) except subprocess.CalledProcessError as error: - print(error.stderr) + logging.error(error.stderr) error_message = ( f"Unable to set ownership of {true_location} :{error.stderr}" )