diff --git a/client/package.json b/client/package.json index b0212963..b34aa888 100644 --- a/client/package.json +++ b/client/package.json @@ -1,6 +1,6 @@ { "name": "client", - "version": "0.7.12", + "version": "0.7.13", "private": true, "scripts": { "build": "vite build", diff --git a/server/.gitignore b/server/.gitignore index cc909312..aab79938 100644 --- a/server/.gitignore +++ b/server/.gitignore @@ -1,9 +1,8 @@ public/ static/ +conf/ digiscript.sqlite digiscript.json -conf/digiscript.sqlite -conf/digiscript.json # Byte-compiled / optimized / DLL files __pycache__/ diff --git a/server/.pylintrc b/server/.pylintrc index 7f8e6ba7..22bfce7c 100644 --- a/server/.pylintrc +++ b/server/.pylintrc @@ -1,5 +1,6 @@ [MASTER] init-hook="from pylint.config import find_pylintrc; import os, sys; sys.path.append(os.path.dirname(find_pylintrc()))" +ignore-paths=^alembic_config/versions/.*$, [MESSAGES CONTROL] disable= diff --git a/server/alembic.ini b/server/alembic.ini new file mode 100644 index 00000000..68a058eb --- /dev/null +++ b/server/alembic.ini @@ -0,0 +1,119 @@ +# A generic, single database configuration. + +[alembic] +# path to migration scripts +script_location = alembic_config + +# template used to generate migration file names; The default value is %%(rev)s_%%(slug)s +# Uncomment the line below if you want the files to be prepended with date and time +# see https://alembic.sqlalchemy.org/en/latest/tutorial.html#editing-the-ini-file +# for all available tokens +# file_template = %%(year)d_%%(month).2d_%%(day).2d_%%(hour).2d%%(minute).2d-%%(rev)s_%%(slug)s + +# sys.path path, will be prepended to sys.path if present. +# defaults to the current working directory. +prepend_sys_path = . + +# timezone to use when rendering the date within the migration file +# as well as the filename. +# If specified, requires the python>=3.9 or backports.zoneinfo library. +# Any required deps can installed by adding `alembic[tz]` to the pip requirements +# string value is passed to ZoneInfo() +# leave blank for localtime +# timezone = + +# max length of characters to apply to the +# "slug" field +# truncate_slug_length = 40 + +# set to 'true' to run the environment during +# the 'revision' command, regardless of autogenerate +# revision_environment = false + +# set to 'true' to allow .pyc and .pyo files without +# a source .py file to be detected as revisions in the +# versions/ directory +# sourceless = false + +# version location specification; This defaults +# to alembic/versions. When using multiple version +# directories, initial revisions must be specified with --version-path. +# The path separator used here should be the separator specified by "version_path_separator" below. +# version_locations = %(here)s/bar:%(here)s/bat:alembic/versions + +# version path separator; As mentioned above, this is the character used to split +# version_locations. The default within new alembic.ini files is "os", which uses os.pathsep. +# If this key is omitted entirely, it falls back to the legacy behavior of splitting on spaces and/or commas. +# Valid values for version_path_separator are: +# +# version_path_separator = : +# version_path_separator = ; +# version_path_separator = space +version_path_separator = os # Use os.pathsep. Default configuration used for new projects. + +# set to 'true' to search source files recursively +# in each "version_locations" directory +# new in Alembic version 1.10 +# recursive_version_locations = false + +# the output encoding used when revision files +# are written from script.py.mako +# output_encoding = utf-8 + +# The relative location of the DigiScript config file to this ini file +digiscript.config = ./conf/digiscript.json +# Whether to configure logging or not +configure_logging = True + + +[post_write_hooks] +# post_write_hooks defines scripts or Python functions that are run +# on newly generated revision scripts. See the documentation for further +# detail and examples + +# format using "black" - use the console_scripts runner, against the "black" entrypoint +# hooks = black +# black.type = console_scripts +# black.entrypoint = black +# black.options = -l 79 REVISION_SCRIPT_FILENAME + +# lint with attempts to fix using "ruff" - use the exec runner, execute a binary +# hooks = ruff +# ruff.type = exec +# ruff.executable = %(here)s/.venv/bin/ruff +# ruff.options = --fix REVISION_SCRIPT_FILENAME + +# Logging configuration +[loggers] +keys = root,sqlalchemy,alembic + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARN +handlers = console +qualname = + +[logger_sqlalchemy] +level = WARN +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S diff --git a/server/alembic_config/README b/server/alembic_config/README new file mode 100644 index 00000000..98e4f9c4 --- /dev/null +++ b/server/alembic_config/README @@ -0,0 +1 @@ +Generic single-database configuration. \ No newline at end of file diff --git a/server/alembic_config/__init__.py b/server/alembic_config/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/server/alembic_config/env.py b/server/alembic_config/env.py new file mode 100644 index 00000000..d14d0503 --- /dev/null +++ b/server/alembic_config/env.py @@ -0,0 +1,106 @@ +import json +import os +from logging.config import fileConfig + +from sqlalchemy import engine_from_config +from sqlalchemy import pool + +from alembic import context + +from models import models + +# this is the Alembic Config object, which provides +# access to the values within the .ini file in use. +config = context.config + +# Interpret the config file for Python logging. +# This line sets up loggers basically. +if config.config_file_name is not None: + if config.get_main_option("configure_logging").lower() == "true": + fileConfig(config.config_file_name) + +# add your model's MetaData object here +# for 'autogenerate' support +models.import_all_models() +target_metadata = models.db.metadata + +# other values from the config, defined by the needs of env.py, +# can be acquired: +# my_important_option = config.get_main_option("my_important_option") +# ... etc. + + +def get_digiscript_db_url(): + rel_path = config.get_main_option("digiscript.config") + if not os.path.isabs(rel_path): + abs_path = os.path.join(os.path.dirname(__file__), "..", rel_path) + else: + abs_path = rel_path + with open(abs_path, "r") as config_file: + ds_config = json.load(config_file) + return ds_config["db_path"] + + +def include_name(name, type_, parent_names): + if type_ == "table": + return name in target_metadata.tables + return True + + +def run_migrations_offline() -> None: + """Run migrations in 'offline' mode. + + This configures the context with just a URL + and not an Engine, though an Engine is acceptable + here as well. By skipping the Engine creation + we don't even need a DBAPI to be available. + + Calls to context.execute() here emit the given string to the + script output. + + """ + url = get_digiscript_db_url() + context.configure( + url=url, + target_metadata=target_metadata, + literal_binds=True, + dialect_opts={"paramstyle": "named"}, + include_schemas=False, + include_name=include_name, + ) + + with context.begin_transaction(): + context.run_migrations() + + +def run_migrations_online() -> None: + """Run migrations in 'online' mode. + + In this scenario we need to create an Engine + and associate a connection with the context. + + """ + connectable = engine_from_config( + config.get_section(config.config_ini_section, {}), + prefix="sqlalchemy.", + poolclass=pool.NullPool, + url=get_digiscript_db_url(), + ) + + with connectable.connect() as connection: + context.configure( + connection=connection, + target_metadata=target_metadata, + include_schemas=False, + include_name=include_name, + render_as_batch=True + ) + + with context.begin_transaction(): + context.run_migrations() + + +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() diff --git a/server/alembic_config/script.py.mako b/server/alembic_config/script.py.mako new file mode 100644 index 00000000..fbc4b07d --- /dev/null +++ b/server/alembic_config/script.py.mako @@ -0,0 +1,26 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision | comma,n} +Create Date: ${create_date} + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +${imports if imports else ""} + +# revision identifiers, used by Alembic. +revision: str = ${repr(up_revision)} +down_revision: Union[str, None] = ${repr(down_revision)} +branch_labels: Union[str, Sequence[str], None] = ${repr(branch_labels)} +depends_on: Union[str, Sequence[str], None] = ${repr(depends_on)} + + +def upgrade() -> None: + ${upgrades if upgrades else "pass"} + + +def downgrade() -> None: + ${downgrades if downgrades else "pass"} diff --git a/server/alembic_config/versions/d4f66f58158b_initial_alembic_revision.py b/server/alembic_config/versions/d4f66f58158b_initial_alembic_revision.py new file mode 100644 index 00000000..19ed2c28 --- /dev/null +++ b/server/alembic_config/versions/d4f66f58158b_initial_alembic_revision.py @@ -0,0 +1,26 @@ +"""Initial Alembic Revision + +Revision ID: d4f66f58158b +Revises: +Create Date: 2024-06-02 15:50:23.550851 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = 'd4f66f58158b' +down_revision: Union[str, None] = None +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + pass + + +def downgrade() -> None: + pass diff --git a/server/digi_server/app_server.py b/server/digi_server/app_server.py index ce2a9deb..285f8655 100644 --- a/server/digi_server/app_server.py +++ b/server/digi_server/app_server.py @@ -1,6 +1,12 @@ import os +import shutil +import time from typing import List, Optional +import sqlalchemy +from alembic import command, script +from alembic.config import Config +from alembic.runtime import migration from tornado.ioloop import IOLoop from tornado.web import Application, StaticFileHandler from tornado_prometheus import PrometheusMixIn @@ -9,8 +15,8 @@ from controllers.ws_controller import WebSocketController from digi_server.logger import get_logger, configure_file_logging, configure_db_logging from digi_server.settings import Settings +from models import models from models.cue import CueType -from models.models import db from models.script import Script from models.show import Show from models.session import Session, ShowSession @@ -18,12 +24,13 @@ from rbac.rbac import RBACController from utils.database import DigiSQLAlchemy from utils.env_parser import EnvParser +from utils.exceptions import DatabaseUpgradeRequired from utils.web.route import Route class DigiScriptServer(PrometheusMixIn, Application): - def __init__(self, debug=False, settings_path=None): + def __init__(self, debug=False, settings_path=None, skip_migrations=False, skip_migrations_check=False): self.env_parser: EnvParser = EnvParser.instance() # pylint: disable=no-member self.digi_settings: Settings = Settings(self, settings_path) @@ -32,12 +39,25 @@ def __init__(self, debug=False, settings_path=None): # Controller imports (needed to trigger the decorator) controllers.import_all_controllers() + # Import all the models + models.import_all_models() self.clients: List[WebSocketController] = [] + self._db: DigiSQLAlchemy = models.db + # Perform database migrations + if not skip_migrations: + self._run_migrations() + else: + get_logger().warning('Skipping performing database migrations') + # And then check the database is up-to-date + if not skip_migrations_check: + self._check_migrations() + else: + get_logger().warning('Skipping database migrations check') + # Finally, configure the database db_path = self.digi_settings.settings.get('db_path').get_value() get_logger().info(f'Using {db_path} as DB path') - self._db: DigiSQLAlchemy = db self._db.configure(url=db_path) self.rbac = RBACController(self) @@ -107,6 +127,45 @@ def log_request(self, handler): return super().log_request(handler) + @property + def _alembic_config(self): + alembic_cfg_path = os.path.join(os.path.dirname(__file__), '..', 'alembic.ini') + alembic_cfg = Config(alembic_cfg_path) + # Override config options with specific ones based on this running instance + alembic_cfg.set_main_option('digiscript.config', self.digi_settings.settings_path) + alembic_cfg.set_main_option('configure_logging', 'False') + return alembic_cfg + + def _run_migrations(self): + try: + self._check_migrations() + except DatabaseUpgradeRequired: + get_logger().info('Running database migrations via Alembic') + # Create a copy of the database file as a backup before performing migrations + db_path: str = self.digi_settings.settings.get('db_path').get_value() + if db_path.startswith('sqlite:///'): + db_path = db_path.replace('sqlite:///', '') + if os.path.exists(db_path) and os.path.isfile(db_path): + get_logger().info('Creating copy of database file as backup') + new_file_name = f'{db_path}.{int(time.time())}' + shutil.copyfile(db_path, new_file_name) + get_logger().info(f'Created copy of database file as backup, saved to {new_file_name}') + else: + get_logger().warning('Database connection does not appear to be a file, cannot create backup!') + # Run the upgrade on the database + command.upgrade(self._alembic_config, 'head') + else: + get_logger().info('No database migrations to perform') + + def _check_migrations(self): + get_logger().info('Checking database migrations via Alembic') + engine = sqlalchemy.create_engine(self.digi_settings.settings.get('db_path').get_value()) + script_ = script.ScriptDirectory.from_config(self._alembic_config) + with engine.begin() as conn: + context = migration.MigrationContext.configure(conn) + if context.get_current_revision() != script_.get_current_head(): + raise DatabaseUpgradeRequired('Migrations required on the database') + async def configure(self): await self._configure_logging() diff --git a/server/main.py b/server/main.py index 1fa4c7f7..5bb44f9c 100755 --- a/server/main.py +++ b/server/main.py @@ -16,13 +16,15 @@ type=str, default=None, help='Path to settings JSON file') +define('skip_migrations', type=bool, default=False, help='skip database migrations') async def main(): parse_command_line() app = DigiScriptServer(debug=options.debug, - settings_path=options.settings_path) + settings_path=options.settings_path, + skip_migrations=options.skip_migrations) await app.configure() app.listen(options.port) diff --git a/server/models/models.py b/server/models/models.py index 34996642..4eb2fde8 100644 --- a/server/models/models.py +++ b/server/models/models.py @@ -1,3 +1,19 @@ +import importlib + +from digi_server.logger import get_logger from utils.database import DigiSQLAlchemy +from utils.pkg_utils import find_end_modules + +IMPORTED_MODELS = {} + + +def import_all_models(): + models = find_end_modules('.', prefix='models') + for model in models: + if model != __name__: + get_logger().debug(f'Importing model module {model}') + mod = importlib.import_module(model) + IMPORTED_MODELS[model] = mod + db = DigiSQLAlchemy() diff --git a/server/pylint-ignore.md b/server/pylint-ignore.md index 8d80aab5..62f68772 100644 --- a/server/pylint-ignore.md +++ b/server/pylint-ignore.md @@ -23,68 +23,235 @@ The recommended approach to using `pylint-ignore` is: # Overview - - [W0613: unused-argument (1x)](#w0613-unused-argument) - - [W1514: unspecified-encoding (2x)](#w1514-unspecified-encoding) + - [E1101: no-member (8x)](#e1101-no-member) + - [W0613: unused-argument (2x)](#w0613-unused-argument) + - [W1514: unspecified-encoding (3x)](#w1514-unspecified-encoding) - [C0103: invalid-name (1x)](#c0103-invalid-name) - [R0205: useless-object-inheritance (1x)](#r0205-useless-object-inheritance) +# E1101: no-member + +## File alembic_config/env.py - Line 14 - E1101 (no-member) + +- `message: Module 'alembic.context' has no 'config' member` +- `author : Tim Bradgate ` +- `date : 2024-06-02T14:57:55` + +``` + 12: # this is the Alembic Config object, which provides + 13: # access to the values within the .ini file in use. +> 14: config = context.config + 15: + 16: # Interpret the config file for Python logging. +``` + + +## File alembic_config/env.py - Line 63 - E1101 (no-member) + +- `message: Module 'alembic.context' has no 'configure' member` +- `author : Tim Bradgate ` +- `date : 2024-06-02T14:57:55` + +``` + 50: def run_migrations_offline() -> None: + ... + 61: """ + 62: url = get_digiscript_db_url() +> 63: context.configure( + 64: url=url, + 65: target_metadata=target_metadata, +``` + + +## File alembic_config/env.py - Line 72 - E1101 (no-member) + +- `message: Module 'alembic.context' has no 'begin_transaction' member` +- `author : Tim Bradgate ` +- `date : 2024-06-02T14:57:55` + +``` + 50: def run_migrations_offline() -> None: + ... + 70: ) + 71: +> 72: with context.begin_transaction(): + 73: context.run_migrations() + 74: +``` + + +## File alembic_config/env.py - Line 73 - E1101 (no-member) + +- `message: Module 'alembic.context' has no 'run_migrations' member` +- `author : Tim Bradgate ` +- `date : 2024-06-02T14:57:55` + +``` + 50: def run_migrations_offline() -> None: + ... + 71: + 72: with context.begin_transaction(): +> 73: context.run_migrations() + 74: + 75: +``` + + +## File alembic_config/env.py - Line 91 - E1101 (no-member) + +- `message: Module 'alembic.context' has no 'configure' member` +- `author : Tim Bradgate ` +- `date : 2024-06-02T14:57:55` + +``` + 76: def run_migrations_online() -> None: + ... + 89: + 90: with connectable.connect() as connection: +> 91: context.configure( + 92: connection=connection, + 93: target_metadata=target_metadata, +``` + + +## File alembic_config/env.py - Line 99 - E1101 (no-member) + +- `message: Module 'alembic.context' has no 'begin_transaction' member` +- `author : Tim Bradgate ` +- `date : 2024-06-02T14:57:55` + +``` + 76: def run_migrations_online() -> None: + ... + 97: ) + 98: +> 99: with context.begin_transaction(): + 100: context.run_migrations() + 101: +``` + + +## File alembic_config/env.py - Line 100 - E1101 (no-member) + +- `message: Module 'alembic.context' has no 'run_migrations' member` +- `author : Tim Bradgate ` +- `date : 2024-06-02T14:57:55` + +``` + 76: def run_migrations_online() -> None: + ... + 98: + 99: with context.begin_transaction(): +> 100: context.run_migrations() + 101: + 102: +``` + + +## File alembic_config/env.py - Line 103 - E1101 (no-member) + +- `message: Module 'alembic.context' has no 'is_offline_mode' member` +- `author : Tim Bradgate ` +- `date : 2024-06-02T14:57:55` + +``` + 101: + 102: +> 103: if context.is_offline_mode(): + 104: run_migrations_offline() + 105: else: +``` + + # W0613: unused-argument -## File controllers/controllers.py - Line 26 - W0613 (unused-argument) +## File controllers/controllers.py - Line 27 - W0613 (unused-argument) - `message: Unused argument 'path'` - `author : Tim Bradgate ` - `date : 2023-01-25T18:15:14` ``` - 24: - 25: class RootController(BaseController): -> 26: def get(self, path): - 27: file_path = os.path.join( - 28: os.path.abspath( + 25: + 26: class RootController(BaseController): +> 27: def get(self, path): + 28: file_path = os.path.join( + 29: os.path.abspath( +``` + + +## File alembic_config/env.py - Line 44 - W0613 (unused-argument) + +- `message: Unused argument 'parent_names'` +- `author : Tim Bradgate ` +- `date : 2024-06-02T14:57:55` + +``` + 42: + 43: +> 44: def include_name(name, type_, parent_names): + 45: if type_ == "table": + 46: return name in target_metadata.tables ``` # W1514: unspecified-encoding -## File controllers/controllers.py - Line 32 - W1514 (unspecified-encoding) +## File controllers/controllers.py - Line 37 - W1514 (unspecified-encoding) - `message: Using open without explicitly specifying an encoding` - `author : Tim Bradgate ` - `date : 2023-01-25T18:15:14` ``` - 26: def get(self, path): + 27: def get(self, path): + ... + 35: raise HTTPError(404) + 36: +> 37: with open(full_path, 'r') as file: + 38: self.write(file.read()) + 39: +``` + + +## File alembic_config/env.py - Line 39 - W1514 (unspecified-encoding) + +- `message: Using open without explicitly specifying an encoding` +- `author : Tim Bradgate ` +- `date : 2024-06-02T14:57:55` + +``` + 33: def get_digiscript_db_url(): ... - 30: "..", - 31: "public") -> 32: with open(os.path.join(file_path, "index.html"), 'r') as file: - 33: self.write(file.read()) - 34: + 37: else: + 38: abs_path = rel_path +> 39: with open(abs_path, "r") as config_file: + 40: ds_config = json.load(config_file) + 41: return ds_config["db_path"] ``` -## File controllers/controllers.py - Line 45 - W1514 (unspecified-encoding) +## File controllers/controllers.py - Line 53 - W1514 (unspecified-encoding) - `message: Using open without explicitly specifying an encoding` - `author : Tim Bradgate ` - `date : 2023-01-25T18:15:14` ``` - 37: def get(self): + 42: def get(self): ... - 43: os.path.sep)) - 44: try: -> 45: with open(full_path, 'r') as file: - 46: self.write(file.read()) - 47: except UnicodeDecodeError: + 51: + 52: try: +> 53: with open(full_path, 'r') as file: + 54: self.write(file.read()) + 55: except UnicodeDecodeError: ``` # C0103: invalid-name -## File controllers/api/settings.py - Line 24 - C0103 (invalid-name) +## File controllers/api/settings.py - Line 26 - C0103 (invalid-name) - `message: Variable name "v" doesn't conform to snake_case naming style` - `author : Tim Bradgate ` @@ -93,11 +260,11 @@ The recommended approach to using `pylint-ignore` is: ``` 11: class SettingsController(BaseAPIController): ... - 22: get_logger().debug(f'New settings data patched: {data}') - 23: -> 24: for k, v in data.items(): - 25: await settings.set(k, v) - 26: + 24: get_logger().debug(f'New settings data patched: {data}') + 25: +> 26: for k, v in data.items(): + 27: await settings.set(k, v) + 28: ``` diff --git a/server/requirements.txt b/server/requirements.txt index 1dac3076..3e8b964c 100644 --- a/server/requirements.txt +++ b/server/requirements.txt @@ -1,9 +1,10 @@ -tornado==6.4 +tornado==6.4.1 tornado-sqlalchemy==0.7.0 datetime==4.9 python-dateutil==2.9.0.post0 pylint-ignore==2022.1025 marshmallow-sqlalchemy==0.30.0 tornado-prometheus==0.1.2 -bcrypt==4.1.2 +bcrypt==4.1.3 anytree==2.12.1 +alembic==1.13.2 diff --git a/server/test/test_utils.py b/server/test/test_utils.py index 6324c099..a5479b38 100644 --- a/server/test/test_utils.py +++ b/server/test/test_utils.py @@ -11,7 +11,8 @@ class DigiScriptTestCase(AsyncHTTPTestCase): def get_app(self): - return DigiScriptServer(debug=True, settings_path=self.settings_path) + return DigiScriptServer(debug=True, settings_path=self.settings_path, + skip_migrations=True, skip_migrations_check=True) def setUp(self): base_path = os.path.join(os.path.dirname(__file__), 'conf') diff --git a/server/utils/exceptions.py b/server/utils/exceptions.py new file mode 100644 index 00000000..3c8af989 --- /dev/null +++ b/server/utils/exceptions.py @@ -0,0 +1,2 @@ +class DatabaseUpgradeRequired(Exception): + pass