From 0a2d62b001eb053235703f4af20cf0c3fcd20516 Mon Sep 17 00:00:00 2001 From: Tiberiu Chibici Date: Fri, 9 Nov 2018 13:40:48 +0200 Subject: [PATCH] Improve error handling for settings. Instead of failing with an exception, an alert is displayed on the main page when something isn't set properly. --- app/YtManager/settings.py | 93 ++++++++++++++----- app/YtManagerApp/scheduler.py | 4 +- .../templates/YtManagerApp/index.html | 2 + .../YtManagerApp/index_errors_banner.html | 24 +++++ .../YtManagerApp/index_unauthenticated.html | 2 + app/YtManagerApp/views/index.py | 14 ++- config/config.ini | 4 +- 7 files changed, 112 insertions(+), 31 deletions(-) create mode 100644 app/YtManagerApp/templates/YtManagerApp/index_errors_banner.html diff --git a/app/YtManager/settings.py b/app/YtManager/settings.py index b1a78fb..ec966e5 100644 --- a/app/YtManager/settings.py +++ b/app/YtManager/settings.py @@ -31,7 +31,7 @@ INSTALLED_APPS = [ 'django.contrib.sessions', 'django.contrib.messages', 'django.contrib.staticfiles', - 'django.contrib.humanize' + 'django.contrib.humanize', ] MIDDLEWARE = [ @@ -126,7 +126,6 @@ CONFIG_DIR = os.path.join(PROJECT_ROOT, "config") DATA_DIR = os.path.join(PROJECT_ROOT, "data") STATIC_ROOT = os.path.join(PROJECT_ROOT, "static") - _DEFAULT_CONFIG_DIR = os.path.join(BASE_DIR, "default") _DEFAULT_CONFIG_FILE = os.path.join(CONFIG_DIR, 'config.ini') _DEFAULT_LOG_FILE = os.path.join(DATA_DIR, 'log.log') @@ -134,6 +133,7 @@ _DEFAULT_MEDIA_ROOT = os.path.join(DATA_DIR, 'media') DEFAULTS_FILE = os.path.join(_DEFAULT_CONFIG_DIR, 'defaults.ini') CONFIG_FILE = os.getenv('YTSM_CONFIG_FILE', _DEFAULT_CONFIG_FILE) +DATA_CONFIG_FILE = os.path.join(DATA_DIR, 'config.ini') # # Defaults @@ -156,37 +156,84 @@ _SCHEDULER_SYNC_SCHEDULE = '5 * * * *' _DEFAULT_SCHEDULER_CONCURRENCY = 1 +CONFIG_ERRORS = [] +CONFIG_WARNINGS = [] + + # # Load globals from config.ini # +def get_global_opt(name, cfgparser, env_variable=None, fallback=None, boolean=False, integer=False): + """ + Reads a configuration option, in the following order: + 1. environment variable + 2. config parser + 3. fallback + + :param name: + :param env_variable: + :param fallback: + :param boolean: + :return: + """ + # Get from environment variable + if env_variable is not None: + value = os.getenv(env_variable) + + if value is not None and boolean: + return value.lower() in ['true', 't', 'on', 'yes', 'y', '1'] + elif value is not None and integer: + try: + return int(value) + except ValueError: + CONFIG_WARNINGS.append(f'Environment variable {env_variable}: value must be an integer value!') + elif value is not None: + return value + + # Get from config parser + if boolean: + try: + return cfgparser.getboolean('global', name, fallback=fallback) + except ValueError: + CONFIG_WARNINGS.append(f'config.ini file: Value set for option global.{name} is not valid! ' + f'Valid options: true, false, on, off.') + return fallback + + if integer: + try: + return cfgparser.getint('global', name, fallback=fallback) + except ValueError: + CONFIG_WARNINGS.append(f'config.ini file: Value set for option global.{name} must be an integer number! ') + return fallback + + return cfgparser.get('global', name, fallback=fallback) + + def load_config_ini(): from configparser import ConfigParser from YtManagerApp.utils.extended_interpolation_with_env import ExtendedInterpolatorWithEnv import dj_database_url cfg = ConfigParser(allow_no_value=True, interpolation=ExtendedInterpolatorWithEnv()) - read_ok = cfg.read([DEFAULTS_FILE, CONFIG_FILE]) + read_ok = cfg.read([DEFAULTS_FILE, CONFIG_FILE, DATA_CONFIG_FILE]) if DEFAULTS_FILE not in read_ok: - print('Failed to read file ' + DEFAULTS_FILE) - raise Exception('Cannot read file ' + DEFAULTS_FILE) - if CONFIG_FILE not in read_ok: - print('Failed to read file ' + CONFIG_FILE) - raise Exception('Cannot read file ' + CONFIG_FILE) + CONFIG_ERRORS.append(f'File {DEFAULTS_FILE} could not be read! Please make sure the file is in the right place,' + f' and it has read permissions.') # Debug global DEBUG - DEBUG = cfg.getboolean('global', 'Debug', fallback=_DEFAULT_DEBUG) + DEBUG = get_global_opt('Debug', cfg, env_variable='YTSM_DEBUG', fallback=_DEFAULT_DEBUG, boolean=True) # Media root, which is where thumbnails are stored global MEDIA_ROOT - MEDIA_ROOT = cfg.get('global', 'MediaRoot', fallback=_DEFAULT_MEDIA_ROOT) + MEDIA_ROOT = get_global_opt('MediaRoot', cfg, env_variable='YTSM_MEDIA_ROOT', fallback=_DEFAULT_MEDIA_ROOT) # Keys - secret key, youtube API key # SECURITY WARNING: keep the secret key used in production secret! global SECRET_KEY, YOUTUBE_API_KEY - SECRET_KEY = cfg.get('global', 'SecretKey', fallback=_DEFAULT_SECRET_KEY) - YOUTUBE_API_KEY = cfg.get('global', 'YoutubeApiKey', fallback=_DEFAULT_YOUTUBE_API_KEY) + SECRET_KEY = get_global_opt('SecretKey', cfg, env_variable='YTSM_SECRET_KEY', fallback=_DEFAULT_SECRET_KEY) + YOUTUBE_API_KEY = get_global_opt('YoutubeApiKey', cfg, env_variable='YTSM_YTAPI_KEY', fallback=_DEFAULT_YOUTUBE_API_KEY) # Database global DATABASES @@ -199,30 +246,32 @@ def load_config_ini(): else: DATABASES['default'] = { - 'ENGINE': cfg.get('global', 'DatabaseEngine', fallback=_DEFAULT_DATABASE['ENGINE']), - 'NAME': cfg.get('global', 'DatabaseName', fallback=_DEFAULT_DATABASE['NAME']), - 'HOST': cfg.get('global', 'DatabaseHost', fallback=_DEFAULT_DATABASE['HOST']), - 'USER': cfg.get('global', 'DatabaseUser', fallback=_DEFAULT_DATABASE['USER']), - 'PASSWORD': cfg.get('global', 'DatabasePassword', fallback=_DEFAULT_DATABASE['PASSWORD']), - 'PORT': cfg.get('global', 'DatabasePort', fallback=_DEFAULT_DATABASE['PORT']), + 'ENGINE': get_global_opt('DatabaseEngine', cfg, env_variable='YTSM_DB_ENGINE', fallback=_DEFAULT_DATABASE['ENGINE']), + 'NAME': get_global_opt('DatabaseName', cfg, env_variable='YTSM_DB_NAME', fallback=_DEFAULT_DATABASE['NAME']), + 'HOST': get_global_opt('DatabaseHost', cfg, env_variable='YTSM_DB_HOST', fallback=_DEFAULT_DATABASE['HOST']), + 'USER': get_global_opt('DatabaseUser', cfg, env_variable='YTSM_DB_USER', fallback=_DEFAULT_DATABASE['USER']), + 'PASSWORD': get_global_opt('DatabasePassword', cfg, env_variable='YTSM_DB_PASSWORD', fallback=_DEFAULT_DATABASE['PASSWORD']), + 'PORT': get_global_opt('DatabasePort', cfg, env_variable='YTSM_DB_PORT', fallback=_DEFAULT_DATABASE['PORT']), } # Log settings global LOG_LEVEL, LOG_FILE - log_level_str = cfg.get('global', 'LogLevel', fallback='INFO') + log_level_str = get_global_opt('LogLevel', cfg, env_variable='YTSM_LOG_LEVEL', fallback='INFO') try: LOG_LEVEL = getattr(logging, log_level_str) except AttributeError: + CONFIG_WARNINGS.append(f'Invalid log level {log_level_str}. ' + f'Valid options are: DEBUG, INFO, WARN, ERROR, CRITICAL.') print("Invalid log level " + LOG_LEVEL) LOG_LEVEL = logging.INFO - LOG_FILE = cfg.get('global', 'LogFile', fallback=_DEFAULT_LOG_FILE) + LOG_FILE = get_global_opt('LogFile', cfg, env_variable='YTSM_LOG_FILE', fallback=_DEFAULT_LOG_FILE) # Scheduler settings global SCHEDULER_SYNC_SCHEDULE, SCHEDULER_CONCURRENCY - SCHEDULER_SYNC_SCHEDULE = cfg.get('global', 'SynchronizationSchedule', fallback=_SCHEDULER_SYNC_SCHEDULE) - SCHEDULER_CONCURRENCY = cfg.getint('global', 'SchedulerConcurrency', fallback=_DEFAULT_SCHEDULER_CONCURRENCY) + SCHEDULER_SYNC_SCHEDULE = get_global_opt('SynchronizationSchedule', cfg, fallback=_SCHEDULER_SYNC_SCHEDULE) + SCHEDULER_CONCURRENCY = get_global_opt('SchedulerConcurrency', cfg, fallback=_DEFAULT_SCHEDULER_CONCURRENCY, integer=True) load_config_ini() diff --git a/app/YtManagerApp/scheduler.py b/app/YtManagerApp/scheduler.py index f1e11b0..4863835 100644 --- a/app/YtManagerApp/scheduler.py +++ b/app/YtManagerApp/scheduler.py @@ -1,19 +1,19 @@ import logging import sys from apscheduler.schedulers.background import BackgroundScheduler +from django.conf import settings scheduler: BackgroundScheduler = None def initialize_scheduler(): - from .appconfig import settings global scheduler logger = logging.getLogger('scheduler') executors = { 'default': { 'type': 'threadpool', - 'max_workers': settings.getint('global', 'SchedulerConcurrency') + 'max_workers': settings.SCHEDULER_CONCURRENCY } } job_defaults = { diff --git a/app/YtManagerApp/templates/YtManagerApp/index.html b/app/YtManagerApp/templates/YtManagerApp/index.html index 6ec5490..9a763b5 100644 --- a/app/YtManagerApp/templates/YtManagerApp/index.html +++ b/app/YtManagerApp/templates/YtManagerApp/index.html @@ -24,6 +24,8 @@ + {% include 'YtManagerApp/index_errors_banner.html' %} +
diff --git a/app/YtManagerApp/templates/YtManagerApp/index_errors_banner.html b/app/YtManagerApp/templates/YtManagerApp/index_errors_banner.html new file mode 100644 index 0000000..f9076d4 --- /dev/null +++ b/app/YtManagerApp/templates/YtManagerApp/index_errors_banner.html @@ -0,0 +1,24 @@ +{% if config_errors %} +
+

Attention! Some critical configuration errors have been found!

+
    + {% for err in config_errors %} +
  • {{ err }}
    • + {% endfor %} +
+

Until these problems are fixed, the server may have encounter serious problems while running. + Please correct these errors, and then restart the server.

+
+{% endif %} + +{% if config_warnings %} +
+

Warning: some configuration problems have been found!

+
    + {% for err in config_warnings %} +
  • {{ err }}
    • + {% endfor %} +
+

We recommend that you fix these issues before continuing.

+
+{% endif %} \ No newline at end of file diff --git a/app/YtManagerApp/templates/YtManagerApp/index_unauthenticated.html b/app/YtManagerApp/templates/YtManagerApp/index_unauthenticated.html index c976a8e..66d1ca0 100644 --- a/app/YtManagerApp/templates/YtManagerApp/index_unauthenticated.html +++ b/app/YtManagerApp/templates/YtManagerApp/index_unauthenticated.html @@ -3,6 +3,8 @@ {% block body %} + {% include 'YtManagerApp/index_errors_banner.html' %} +

Hello

Please log in to continue

diff --git a/app/YtManagerApp/views/index.py b/app/YtManagerApp/views/index.py index d18fb59..f7cfd7a 100644 --- a/app/YtManagerApp/views/index.py +++ b/app/YtManagerApp/views/index.py @@ -8,7 +8,7 @@ from django.http import HttpRequest, HttpResponseBadRequest, JsonResponse from django.shortcuts import render from django.views.generic import CreateView, UpdateView, DeleteView, FormView from django.views.generic.edit import FormMixin - +from django.conf import settings from YtManagerApp.management.videos import get_videos from YtManagerApp.models import Subscription, SubscriptionFolder, VIDEO_ORDER_CHOICES, VIDEO_ORDER_MAPPING from YtManagerApp.utils import youtube, subscription_file_parser @@ -94,13 +94,17 @@ def __tree_sub_id(sub_id): def index(request: HttpRequest): + context = { + 'config_errors': settings.CONFIG_ERRORS, + 'config_warnings': settings.CONFIG_WARNINGS, + } if request.user.is_authenticated: - context = { - 'filter_form': VideoFilterForm() - } + context.update({ + 'filter_form': VideoFilterForm(), + }) return render(request, 'YtManagerApp/index.html', context) else: - return render(request, 'YtManagerApp/index_unauthenticated.html') + return render(request, 'YtManagerApp/index_unauthenticated.html', context) @login_required diff --git a/config/config.ini b/config/config.ini index 91352d4..b7569ab 100644 --- a/config/config.ini +++ b/config/config.ini @@ -4,7 +4,7 @@ ; The global section contains settings that apply to the entire server [global] -Debug=${env:YTSM_DEBUG} +;Debug=False ; This is the folder where thumbnails will be downloaded. By default project_root/data/media is used. ;MediaRoot= @@ -42,7 +42,7 @@ Debug=${env:YTSM_DEBUG} ; Number of threads running the scheduler ; Since most of the jobs scheduled are downloads, there is no advantage to having ; a higher concurrency -;SchedulerConcurrency=1 +SchedulerConcurrency=a1a ; Default user settings [user]